Skip to content

[MNG-7559] Fix versions comparison - #845

Merged
elharo merged 1 commit into
apache:masterfrom
sultan:comparison
Dec 3, 2022
Merged

elharo merged 1 commit into
apache:masterfrom
sultan:comparison

Conversation

@sultan

@sultan sultan commented Oct 22, 2022 •

Copy link
Copy Markdown
Contributor

Fix versions comparison https://issues.apache.org/jira/browse/MNG-7559
intention is:

  • 1.0.0.RC1 < 1.0.0-RC2
  • ( edr, pfd, etc.) < final, ga, release
  • 9.4.1.jre16 > 9.4.1.jre16-preview

following semver rules should be encouraged, natural ordering is used without the need to hard code strings, except for hard coded qualifiers 'a', 'b', 'm', 'cr', 'snapshot', 'final', 'ga', 'release', '' and 'sp':

  • alpha = a < beta = b < milestone = m < rc = cr < 'snapshot' < '' = 'final' = 'ga' = 'release' < 'sp'

the documentation should discourage the usage of 'CR', 'final', 'ga', 'release' and 'SP' qualifiers.
Maven Central should begin to reject new artifact using CR and SP qualifiers.


Following this checklist to help us incorporate your
contribution quickly and easily:

  • Make sure there is a JIRA issue filed
    for the change (usually before you start working on it). Trivial changes like typos do not
    require a JIRA issue. Your pull request should address just this issue, without
    pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [MNG-XXX] SUMMARY, where you replace MNG-XXX
    and SUMMARY with the appropriate JIRA issue. Best practice is to use the JIRA issue
    title in the pull request title and in the first line of the commit message.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean verify to make sure basic checks pass. A more thorough check will
    be performed on your pull request automatically.
  • You have run the Core IT successfully.

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

@sultan sultan changed the title Fix versions comparison |MNG-7559] Fix versions comparison Oct 22, 2022
@sultan sultan changed the title |MNG-7559] Fix versions comparison [MNG-7559] Fix versions comparison Oct 22, 2022
@sultan
sultan force-pushed the comparison branch 2 times, most recently from 194316a to 9372f04 Compare October 22, 2022 18:35
@sultan
sultan marked this pull request as ready for review November 6, 2022 11:35
@sultan

sultan commented Nov 6, 2022 •

Copy link
Copy Markdown
Contributor Author

made the items attribute private with a public getter to respect the code standard

@sultan
sultan marked this pull request as draft November 6, 2022 12:28
@sultan
sultan force-pushed the comparison branch 2 times, most recently from 83a0828 to 8e382dd Compare November 6, 2022 14:42
@sultan

sultan commented Nov 6, 2022

Copy link
Copy Markdown
Contributor Author

added regression test cases

@sultan
sultan marked this pull request as ready for review November 6, 2022 14:43
@sultan
sultan marked this pull request as draft November 6, 2022 15:12
@sultan
sultan marked this pull request as ready for review November 6, 2022 16:56
@sultan
sultan marked this pull request as draft November 12, 2022 20:12
@sultan
sultan marked this pull request as ready for review November 12, 2022 21:08
@sultan

sultan commented Nov 12, 2022

Copy link
Copy Markdown
Contributor Author

@sultan

sultan commented Nov 12, 2022

Copy link
Copy Markdown
Contributor Author

@sultan
sultan force-pushed the comparison branch 2 times, most recently from e467f85 to 9b33d69 Compare November 13, 2022 13:16
@sultan

sultan commented Nov 13, 2022

Copy link
Copy Markdown
Contributor Author

added another test case to prove website documentation right

@sultan

sultan commented Nov 13, 2022

Copy link
Copy Markdown
Contributor Author

code and documentation on par. waiting for final reviews.

@elharo elharo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm still digging into the other PRs and the existing spec and code. Pleas ebear with me. It's been a while since I looked at this, and I need to make sure all the fiddly little bits are straight in my head.

elharo
elharo previously requested changes Nov 14, 2022

@elharo elharo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Check the output of the CI. The test failures look related.

@sultan

sultan commented Nov 14, 2022

Copy link
Copy Markdown
Contributor Author

made 3 changes.

  • javadoc without the last dot
  • put back a method that returns a string
  • fixed test, hopefully

@sultan

sultan commented Nov 29, 2022 •

Copy link
Copy Markdown
Contributor Author

@elharo, rebased and applied code styles with spotless plugin

@sultan
sultan requested a review from elharo November 29, 2022 12:13
@sultan
sultan force-pushed the comparison branch 2 times, most recently from 7b078b3 to 8bd408c Compare November 29, 2022 12:39
@elharo
elharo dismissed their stale review November 29, 2022 12:41

rereviewing

@elharo elharo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, just some doc nits

@sultan

sultan commented Nov 29, 2022

Copy link
Copy Markdown
Contributor Author

made the suggested change, thanks @elharo

@sultan

sultan commented Nov 29, 2022 •

Copy link
Copy Markdown
Contributor Author

@elharo, fixed javadoc build errors by replacing > with &gt; and < with &lt;

@elharo

elharo commented Dec 3, 2022

Copy link
Copy Markdown
Contributor

All looks good. I need to copy the PR into my account to run it through Jenkins

@elharo elharo mentioned this pull request Dec 3, 2022
@elharo

elharo commented Dec 3, 2022

Copy link
Copy Markdown
Contributor

Jenkins passed my fork of this so I'm going to merge.

https://ci-maven.apache.org/job/Maven/job/maven-box/job/maven/job/MNG-7559/

@elharo
elharo merged commit 97e8bf6 into apache:master Dec 3, 2022
@gnodet gnodet added this to the 4.0.0-alpha-3 milestone Dec 5, 2022
@sultan

sultan commented Dec 16, 2022

Copy link
Copy Markdown
Contributor Author

@elharo is it wished to port this to 3.9.x branch? if so a PR is available here:

@elharo

elharo commented Dec 16, 2022

Copy link
Copy Markdown
Contributor

Check on the dev mailing list and see of anyone is planning another 3.9 release.

asfgit pushed a commit that referenced this pull request Dec 19, 2022
@michael-o

Copy link
Copy Markdown
Member

This merge has been reverted on master.

@sultan

sultan commented Dec 20, 2022

Copy link
Copy Markdown
Contributor Author

This merge has been reverted on master.

A new clean PR was created to reintroduce the fix

@jira-importer

Copy link
Copy Markdown

Resolve #8891

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.

6 participants