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

Use nested module format for Java package names #2236

Closed
wants to merge 1 commit into from

Conversation

kalenp
Copy link
Contributor

@kalenp kalenp commented Oct 21, 2024

JRuby provides two formats for referring to java package [1]. Sequel has historically used a mix of both formats. This consolidates on the "nested module" format (Java::JavaSql::Date) vs the java style format (java.sql.Date). Apart from internal consistency, this format also presents as more idiomatic Ruby, since there are no invalid module names, such as Java::oracle.jdbc.driver.OracleDriver (oracle is not a valid module name).

[1] https://github.com/jruby/jruby/wiki/CallingJavaFromJRuby#referencing-java-classes-using-full-qualified-class-name

JRuby provides two formats for referring to java package [1].  Sequel
has historically used a mix of both formats.  This consolidates on the
"nested module" format (Java::JavaSql::Date) vs the java style format
(java.sql.Date).  Apart from internal consistency, this format also
presents as more idiomatic Ruby, since there are no invalid module
names, such as Java::oracle.jdbc.driver.OracleDriver (`oracle` is not a
valid module name).

[1] https://github.com/jruby/jruby/wiki/CallingJavaFromJRuby#referencing-java-classes-using-full-qualified-class-name

Change-Id: I76a6786e011b4b54bfc96bb3c9bb288875d51bd6
@kalenp
Copy link
Contributor Author

kalenp commented Oct 21, 2024

My use case is only affected by the OracleTypes declarations, but I figured it would be worth covering the rest for consistency sake. I can reduce the scope of this if you like.

@jeremyevans
Copy link
Owner

Thanks for submitting a patch for this and fixing all cases. I probably will not have time to test this until this coming weekend, but as long as it passes all jdbc driver tests, I will merge it.

@jeremyevans
Copy link
Owner

All tests passed, so I'll merge this shortly.

@jeremyevans
Copy link
Owner

Cherry-picked at bba3e40

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.

2 participants