Skip to content

Conversation

@andreaTP
Copy link
Collaborator

This will enable compile time checks of the versions in the Quarkus Operator SDK.
ref: quarkiverse/quarkus-operator-sdk#460

cc. @metacosm

@metacosm
Copy link
Collaborator

@andreaTP
Copy link
Collaborator Author

@metacosm I took a look and the scope of Version seems to be quite different, as well as the way to achieve it.
Do you have something in mind?

cc. @csviri

@metacosm
Copy link
Collaborator

@metacosm I took a look and the scope of Version seems to be quite different, as well as the way to achieve it. Do you have something in mind?

cc. @csviri

What I meant is that all these versions should be accessible via Version… We could probably keep the current Versions however, it shouldn't be public, imo, and its information accessed via the Version API (which is also reused by the Quarkus extension).

@andreaTP
Copy link
Collaborator Author

@metacosm I have attempted to follow your comment, unsure if this is what you meant ...


private Versions() {}

protected static final String JOSDK = "${project.version}";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you pls enlighten me how this supposed to work? Where are those placeholders filled?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This file is filled by the Maven Plugin as it is in java-templates folder as opposed to java.

Copy link
Collaborator

Choose a reason for hiding this comment

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

would it be possible to write a unit test for this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done here: 52643d3

Copy link
Collaborator

Choose a reason for hiding this comment

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

great, thx!

</configuration>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To fill in the information that is available in Maven.

Copy link
Collaborator

@csviri csviri left a comment

Choose a reason for hiding this comment

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

I have no objections, if this helps with the quarkus extension fine by me.

@metacosm
Copy link
Collaborator

metacosm commented Jan 5, 2023

@andreaTP could you give me access to push changes to your branch, please? Or would you prefer I open a different PR based on this one with my changes?

@andreaTP
Copy link
Collaborator Author

andreaTP commented Jan 5, 2023

@metacosm as you prefer, you have an invite for my fork or you can open another PR to target the improvements or whatever.
No strong opinions at all! 👍

@metacosm
Copy link
Collaborator

metacosm commented Jan 6, 2023

Didn't easily manage to push changes to the original branch so created a new PR: #1706.

@metacosm metacosm closed this Jan 6, 2023
@andreaTP
Copy link
Collaborator Author

andreaTP commented Jan 6, 2023

No worries, thanks @metacosm !

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.

3 participants