Skip to content
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

Migrate plugin to use dry-struct from virtus #36

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mpiggott
Copy link

Hi, while attempting to upgrade Nexus Repository Manager 2 to a newer version of JRuby I ran into some issues caused by axiom-types (a dependency of virtus) it appears it does not work on newer versions of Ruby. It appears that that library has been abandoned by its authors (a PR fixing it was opened 5-years ago).

I'll lead with - I'm not a ruby programmer, I've never touched it before but I've made an attempt here to switch from the also deprecated virtus to its successor dry-struct. AFAICT in the context of Nexus my changes work.

However dry-struct inserts fields into a Hash thus doesn't maintain order and results in POMs whose elements are in an arbitrary order. With my limited knowledge of ruby I wasn't able to figure out a way to iterate over the fields in the defined order, though I suspect its possible.

The obvious issue here is that this leads to test failures, I have other changes which attempt to resolve that by making the expected test results match the arbitrary ordering though I wasn't able to resolve all the issues (the expected output is used for multiple tests which generate different arbitrary ordering)

@@ -20,6 +20,6 @@
#
module Maven
module Tools
VERSION = '1.1.7'.freeze
VERSION = '1.2.0'.freeze
Copy link
Author

Choose a reason for hiding this comment

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

I figured 1.2 due to the dependency changes

Comment on lines +31 to +37
s.add_runtime_dependency 'dry-container', '~> 0.7.2'
s.add_runtime_dependency 'dry-configurable', '~> 0.12.1'
s.add_runtime_dependency 'dry-core', '~> 0.6.0'
s.add_runtime_dependency 'dry-inflector', '~> 0.2.0'
s.add_runtime_dependency 'dry-types', '~> 1.5.1'
s.add_runtime_dependency 'dry-struct', '~> 1.4.0'
s.add_runtime_dependency 'dry-struct-setters', '~> 0.4.0'
Copy link
Author

Choose a reason for hiding this comment

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

These were the dependency versions whose releases worked on the version of Ruby in the JRuby dependency

@@ -19,17 +19,8 @@
</scm>
<repositories>
<repository>
<id>rubygems</id>
<name>Rubygems Proxy</name>
<url>http://rubygems-proxy.torquebox.org/releases</url>
Copy link
Author

Choose a reason for hiding this comment

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

The torquebox domain lapsed recently

<type>gem</type>
</dependency>
<dependency>
<groupId>rubygems</groupId>
<artifactId>rake</artifactId>
<version>[10.0,10.99999]</version>
Copy link
Author

Choose a reason for hiding this comment

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

Version ranges didn't seem to work with wagon

Maven::Tools::Artifact.from_coordinate( 'sdas:das:jar:123:[de:fr,gb:us]' ).to_s.must_equal 'sdas:das:jar:123:[de:fr,gb:us]'
Maven::Tools::Artifact.from_coordinate( 'sdas:das:jar:tes:[123,234]:[de:fr,gb:us]' ).to_s.must_equal 'sdas:das:jar:tes:[123,234]:[de:fr,gb:us]'
Maven::Tools::Artifact.from_coordinate( 'sdas:das:jar:[123,234]:[de:fr,gb:us]' ).to_s.must_equal 'sdas:das:jar:[123,234]:[de:fr,gb:us]'
_(Maven::Tools::Artifact.from_coordinate( 'sdas:das:jar:tes:123' ).to_s).must_equal 'sdas:das:jar:tes:123'
Copy link
Author

Choose a reason for hiding this comment

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

minitest was complaining about deprecated calls

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.

1 participant