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

feat(#324): Programs.version() #325

Merged
merged 5 commits into from
Feb 12, 2025
Merged

feat(#324): Programs.version() #325

merged 5 commits into from
Feb 12, 2025

Conversation

h1alexbel
Copy link
Contributor

In this PR I've added version() method to Programs to read lints version from MANIFEST.MF.

closes #324

@h1alexbel
Copy link
Contributor Author

@maxonfjvipon please check

MatcherAssert.assertThat(
"Version doesn't match with expected",
new Programs(new ListOf<>()).version(),
Matchers.equalTo("1.2.3")
Copy link
Member

Choose a reason for hiding this comment

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

@h1alexbel is it correct value for testing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maxonfjvipon yes, it comes from src/test/resources/META-INF/MANIFEST.MF:

@h1alexbel
Copy link
Contributor Author

h1alexbel commented Feb 11, 2025

@yegor256 take a look, please

Copy link
Member

@yegor256 yegor256 left a comment

Choose a reason for hiding this comment

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

@h1alexbel looks cool, see one comment above

* @return Version
* @checkstyle NonStaticMethodCheck (3 lines)
*/
public String version() {
Copy link
Member

Choose a reason for hiding this comment

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

@h1alexbel it seems that this method should be static (as checkstyle suggests). However, we don't appreciate static methods. I suggest getting rid of it at all and moving this information into Javadoc. Just explain to the user how he get get the version of the library, via Manifests

@yegor256 yegor256 merged commit 403a418 into objectionary:master Feb 12, 2025
16 checks passed
@yegor256
Copy link
Member

@h1alexbel thanks!

@h1alexbel h1alexbel deleted the 324 branch February 12, 2025 08:35
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.

Programs.version is needed
3 participants