-
Notifications
You must be signed in to change notification settings - Fork 267
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
#632 Fixed lower and upper bounds to keep milestones and rcs in the right majors. #672
Conversation
ada7f3b
to
9a0a4de
Compare
basically this PR adds a "forceSnapshot" boolean to the increaseSegment method. this allows to create bounds that includes 4.0-snaphost 4.0-rc 4.0-m1 4.0-final in the same 4.0 major, for that we need that boolean to increase 3.0 into lower bound 4.0-snaphost included, and upper bound 5.0-snapshot excluded. another PR is on the way to basically ease the exclusions of such "previews" builds. |
All good. No more job needed on my side for this PR. Agreeing on removing unused method, or a cleanup/refactoring. |
I can’t have access to my computer this morning. If you can merge both today I’m fine. Though I can help a bit later |
Sure. If you still want to sync, let me know, we can merge our branches. |
I'll try something in about 6 hours if it can help |
If you go and check in my branch, |
great job of factorization. i'll see if i can merge all of it in my PR tomorrow |
c2a23ed
to
2fc0d46
Compare
rebased and ready. |
if you need to merge #708 first, then there is room for me to improve my PR, it seems the forceSnaphsot will be always true after all. then i can simplify everything. and maybe rebase my PR onto master after 707 and 708 are merged ? |
Thanks ! can Mercury versions allow snapshot qualifier ? is it planned to drop support for Mercury ? |
rebased but requires your attention on above questions before merging |
Looks like it did support -SNAPSHOT and similar qualifiers. https://www.mail-archive.com/[email protected]/msg77826.html https://www.mail-archive.com/[email protected]/msg76344.html And there's also https://github.com/apache/maven-mercury Maybe it's best to ask @olamy ? Version is specified in https://docs.osgi.org/download/r4v41/r4.core.pdf, §3.2.4 "Version" |
Thanks ! the PR currently generates bounds with a "-SNAPSHOT" qualifier, always, regardless of comparator (Maven, Mercury, NumericVersion). the current result is tested against all three Test classes which I modified to expect always the snapshot qualifier. if these Tests classes are okay for the reviewers, then the PR can be merged. |
In the latest picture we have:
Should 4.0.0-RC2 will be Next Major instead of 4.0.0-RC1? |
Not sure about differentiating RC1 vs RC2. the question of differentiating RC1 vs M1 however is another issue that may be adressed in another PR. the problem is caused by dashes and dots not treated the same way. The result could better be M1 Next Major followed by RC1 RC2. this problem was already there before the PR. I know the involved class but I did not investigate the solution. The improvement done in this PR is having M1 RC1 RC2 and Realease labelled in the same major. I'm open to improvement if you can provide a textual expected rendering. |
ace04fa
to
4b81f62
Compare
…in the right majors.
4b81f62
to
0062205
Compare
thanks @slawekjaranowski for the merge with the master branch. as a result of fixing #632, it appears we could maybe get rid of the class MajorMinorIncrementalFilter which states that its very existence is filtering milestones and RC from the result of the calls, when they are in the wrong major, and are now already filtered out by my PR. meanwhile this PR is still ready for review/merge. Thank you for your time. |
#632 Fix Bounds so lower and upper bounds keep milestones and rcs in the same majors when they are.
a quick before/after :