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

Adopt pure ruby DSL implementation for JRuby #9047

Merged
merged 8 commits into from Oct 5, 2021

Conversation

JasonLunn
Copy link
Contributor

JRuby unit and conformance test fixes.

Includes changes originally submitted in #8997 and #8954

JRuby unit and conformance test fixes.
Copy link
Contributor

@perezd perezd left a comment

Choose a reason for hiding this comment

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

LGTM!

ruby/pom.xml Outdated
@@ -88,7 +88,7 @@
<dependency>
<groupId>com.google.protobuf</groupId>
<artifactId>protobuf-java-util</artifactId>
<version>3.13.0</version>
<version>3.18.0</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you'll need to add an entry to this:
https://github.com/protocolbuffers/protobuf/blob/master/update_version.py

to ensure this stays updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done - thanks for the pointer.

raise ArgumentError, "maven needs to be installed"
end
task :clean do
task :clean => :require_mvn do
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to require Maven for this? Could we just run mvn only when it is present?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The JRuby gem assembly process has a dependency on Maven - they just don't need to be enforced unless you're actually running one of the JRuby tasks; previously it was enforcing the availability of Maven when the tasks were defined.

end

require 'google/protobuf/descriptor_dsl'

Copy link
Member

Choose a reason for hiding this comment

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

rm blank line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@JasonLunn JasonLunn merged commit d546ec7 into protocolbuffers:master Oct 5, 2021
@JasonLunn JasonLunn deleted the jruby_test_fixes_redux branch October 5, 2021 18:13
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.

None yet

4 participants