-
Notifications
You must be signed in to change notification settings - Fork 18
[MSHARED-1043] Deprecate methods leveraging JUnit4 assertions #9
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
[MSHARED-1043] Deprecate methods leveraging JUnit4 assertions #9
Conversation
2133b8e to
eb1d5e0
Compare
elharo
left a comment
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.
some nits
24c7c7e to
c6a0216
Compare
slawekjaranowski
left a comment
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.
It will be appreciated to add javadocs for new public methods.
Especially that you link new methods in deprecated ones.
c6a0216 to
f535895
Compare
f535895 to
7403801
Compare
elharo
left a comment
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.
I like this idea overall. I'm a little concerned that this is somewhat incompatible with existing usage. What other code outside this project uses the Verifier class?
7403801 to
914b599
Compare
This code should be fully backwards-compatible as no existing methods are removed/modified and only new ones are added. Some consumers can be seen at https://mvnrepository.com/artifact/org.apache.maven.shared/maven-verifier/usages. In any case a migration away from the deprecated methods should be pretty straightforward. |
|
It's more of a long term concern. Is anyone who's not the Maven project using this? If so, it's going to be really hard to remove the deprecated methods. I agree that if we had a green field this approach is preferable. |
|
It is standard way, first deprecated method with documentation how should be replace - and we do this step. Next we release major version with removed methods in some time. |
|
It's the standard way, assuming step 2 can in fact be done. Far too often, step 2 never happens and the deprecated code is never removed and supported forever. |
|
Thanks for the link to mvnrepository. That's more usages than I would have guessed. My gut is that we're not going to remove these methods. Too many projects depend on them already. I think we're better off living with what we have. |
|
We shouldn't stop development based on simple usage. Eg. now many project migrate to JUnit 5 or use other test frameworks than JUnit 4. In my feel many projects do not upgrade their dependency, so it is their decision. |
|
Better stats is usage per version, like: |
slawekjaranowski
left a comment
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.
please use deprecated javadoc tag and annotation together.
fabe2a4 to
22a7de0
Compare
22a7de0 to
4bf4d80
Compare
Provide alternatives relying on framework agnostic exception instead
4bf4d80 to
ca75aff
Compare
I think everything should be resolved now. |
|
Resolve #150 |
1 similar comment
|
Resolve #150 |
Provide alternatives relying on framework agnostic exception instead