Skip to content

Removal of Mercury and Numeric version schemes, refactoring#1212

Closed
andrzejj0 wants to merge 2 commits intomojohaus:masterfrom
andrzejj0:rules-refactor
Closed

Removal of Mercury and Numeric version schemes, refactoring#1212
andrzejj0 wants to merge 2 commits intomojohaus:masterfrom
andrzejj0:rules-refactor

Conversation

@andrzejj0
Copy link
Copy Markdown
Contributor

Removed Numeric and Mercury version schemes. Mercury is probably not used at all, Numeric can be used due to some artifacts, but since it is not respected by Maven Resolver itself, it makes little sense to keep it.

With the two comparators removed, we can simply use the default comparator that DefaultArtifact provides on its own (which is also in line with Resolver implementation of GenericVersion).

The rest is refactoring:

  • Simplified code, added some unit tests while doing so;
  • Extracted RuleService from VersionsHelper to handle rules-related business, e.g. ignored versions;
  • Extracted ArtifactFactory;
  • Renamed DefaultArtifactVersionCache to ArtifactVersionService;
  • PropertyVersions: moved artifact resolution business to DefaultVersionsHelper;

@slawekjaranowski
Copy link
Copy Markdown
Member

great, as it is next big change I will try to review it in next weekend
of course will be good if someone else also look at it

@andrzejj0
Copy link
Copy Markdown
Contributor Author

Please feel free to invite other reviewers. Maybe Hervé or Tamas?

* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
* under the License.T
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

😄

@slawekjaranowski
Copy link
Copy Markdown
Member

can you split for smaller one ... eg, looks like can be done separatly

  • Extracted ArtifactFactory;
  • Renamed DefaultArtifactVersionCache to ArtifactVersionService;

@andrzejj0
Copy link
Copy Markdown
Contributor Author

can you split for smaller one ... eg, looks like can be done separatly

* Extracted ArtifactFactory;

* Renamed DefaultArtifactVersionCache to ArtifactVersionService;

Created a PR including only those two, but it's still quite sizeable mostly due to the fact that we're refactoring a key part of this whole plugin -- even though the change is small.

@andrzejj0 andrzejj0 force-pushed the rules-refactor branch 3 times, most recently from 86fd814 to 257dc2c Compare March 11, 2025 20:36
@andrzejj0 andrzejj0 force-pushed the rules-refactor branch 4 times, most recently from 44167a1 to 37f81eb Compare March 15, 2025 07:53
@andrzejj0 andrzejj0 closed this Mar 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants