-
Notifications
You must be signed in to change notification settings - Fork 4
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
Conversation
@maxonfjvipon please check |
MatcherAssert.assertThat( | ||
"Version doesn't match with expected", | ||
new Programs(new ListOf<>()).version(), | ||
Matchers.equalTo("1.2.3") |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
:
Lints-Version: 1.2.3 |
@yegor256 take a look, please |
There was a problem hiding this 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() { |
There was a problem hiding this comment.
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
@h1alexbel thanks! |
In this PR I've added
version()
method toPrograms
to read lints version fromMANIFEST.MF
.closes #324