Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#258] Refactor @Parameter to include iri instead of (urlPrefix, name) #273

Merged
merged 5 commits into from
Sep 13, 2024

Conversation

blcham
Copy link
Contributor

@blcham blcham commented Sep 12, 2024

Fixes #258

@blcham
Copy link
Contributor Author

blcham commented Sep 12, 2024

Build fails due to:
image

Note it is related to mvn module:

  <name>SPipes Module Annotation Processor</name>
  <artifactId>s-pipes-module-creator-maven-plugin</artifactId>

@palagdan
Copy link
Collaborator

palagdan commented Sep 13, 2024

@blcham
There is an error here because the annotation has only a 'comment' parameter in CKAN module, but it should also include an 'iri' parameter. I'll try to investigate.
Screenshot from 2024-09-13 06-43-09

I don't understand. readConstraintsFromClass traverses the fields and retrieves the annotations, but the annotations already lack the 'iri' field...
Screenshot from 2024-09-13 07-14-49

Maybe the issue is due to an outdated dependency in the s-pipes-module-creator-maven-plugin, which uses a previous implementation of @Parameter with urlPrefix and name?
:

  <dependency>
            <groupId>cz.cvut.kbss</groupId>
            <artifactId>s-pipes-core</artifactId>
        </dependency>

@blcham
Copy link
Contributor Author

blcham commented Sep 13, 2024

@palagdan

Thanks for the hint, I commit-ed fix of the issue ... not going to search which was the biggest issue but I assume the most important part of what I did was:
image

Please review the PR.

@blcham blcham requested a review from palagdan September 13, 2024 09:11
@blcham blcham force-pushed the 258-refactor-parameter-annotation-to-use-iri branch from 7911ddd to 2e23bd7 Compare September 13, 2024 10:24
@blcham blcham force-pushed the 258-refactor-parameter-annotation-to-use-iri branch from 2e23bd7 to c1d2865 Compare September 13, 2024 10:53
Copy link
Collaborator

@palagdan palagdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reviewed and noticed only one issue with raw values. I will do this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using constants instead of raw values here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using constants instead of raw values here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using constants instead of raw values here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using constants instead of raw values here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using constants instead of raw values here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using constants instead of raw values here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using constants instead of raw values here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using constants instead of raw values here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using constants instead of raw values here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using constants instead of raw values here.

@blcham
Copy link
Contributor Author

blcham commented Sep 13, 2024

Suggestions above will be resolved in subsequent task -- #275

@blcham blcham merged commit 9a92cae into main Sep 13, 2024
2 checks passed
@blcham blcham deleted the 258-refactor-parameter-annotation-to-use-iri branch September 13, 2024 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add constants instead of raw values to attribute name in @parameter annotation
2 participants