-
Notifications
You must be signed in to change notification settings - Fork 192
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
fix #2877 #2878
fix #2877 #2878
Conversation
@@ -17,9 +17,9 @@ | |||
import org.osgi.framework.Version; | |||
|
|||
public class Versions { | |||
private static final String SUFFIX_QUALIFIER = ".qualifier"; | |||
public static final String SUFFIX_QUALIFIER = ".qualifier"; |
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.
We probably should move those constants to the TychoConstants class.
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 can do this in another PR. Would you backport this PR to 4.0.x first?
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.
Just add it to this pr no need for several ones. It would also be good to improve the commit message a bit (e.g. describe what is the problem and what was changed).
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.
Will do.
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.
Looks good, a testcase would be good to ensure this will not breakin the future.
Where is a good place to add a testcase? |
@mlehmannm we have some integration test for the version plugin here: |
Providing a testcase is really hard, because one would need a repository with at least an eclipse-plugin and the same eclipse-plugin as source with changes so that the baseline-mojo needs to increment the version of that eclipse-plugin. |
You might want to take a look at the BaselineMojoTest that creates a baseline repo first (you might even want to add your testcase there) and then modify the source sightly to generate a baseline error. |
I'll try to add a test to BaselineMojoTest next week. |
@mlehmannm whats the status here? I plan to publish a bugfix of Tycho4 soon and it seems good to have your fix included. |
Sorry for the delay. I was on vacation and had a busy week afterwards. I plan to provide a test for the fix, but it might take some time, but I think you can include that fix as it is mostly trivial. |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
No description provided.