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

IE-517 Ruby 3.0.6 Compatibility #8

Merged
merged 7 commits into from
Feb 5, 2024
Merged

IE-517 Ruby 3.0.6 Compatibility #8

merged 7 commits into from
Feb 5, 2024

Conversation

ncperry
Copy link

@ncperry ncperry commented Feb 2, 2024

In order to make Ruby 3 compatible we had to:

  • Add the rake gem as an explicit dependency
  • Update the omni-auth dependency
  • Provide an explicit xml parser. Evidently choosing an xml parser is now up to the developer. For more details see the commit: ea1c3d0

For CI and maintenance we:

  • Added a github rspec action
  • Added a devcontainer file. Such a file specifies a docker image where we know that the gem will build. The next developer to work in this area simply has to stand up a docker container, run bundle install, and they're guaranteed that their environment will be properly setup. I want to experiment with this approach to see if it will ease maintenance for the non-trivial number of gems that we support for Konekti.

https://kantata.atlassian.net/browse/IE-517

Starting in 2013, gem-ified standard library gems were only made available
through Bundler if the gem was specified in the Gemfile, see:
sferik/multi_xml#42

As a result we started to get the error: "No XML parser detected. If you're using Rubinius and Bundler, try adding an XML parser to your Gemfile (e.g. libxml-ruby, nokogiri, or rubysl-rexml). For more information, see sferik/multi_xml#42."

To fix this issue we need to specify an xml parser in our Gemfile. We're not going to be doing that much work within an auth Gem, so performance of the xml parser doesn't matter that much. But we picked libxml for performance anyways. (https://medium.com/neuronio/generating-xml-in-ruby-builder-vs-nokogiri-vs-libxml-vs-ox-757ce2d8f83c)
@@ -1,5 +1,5 @@
module OmniAuth
module Concur
VERSION = "0.0.1"
VERSION = "0.0.2"
Copy link
Author

Choose a reason for hiding this comment

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

This one is for you @wilkinsondesouza

Choose a reason for hiding this comment

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

I appreciate that my rambling is being remembered 😄

@ncperry ncperry changed the title Ie 517 ruby 3 0 6 Ie 517 Ruby 3.0.6 Compatibility Feb 5, 2024
@ncperry ncperry changed the title Ie 517 Ruby 3.0.6 Compatibility Ie-517 Ruby 3.0.6 Compatibility Feb 5, 2024
@@ -1,5 +1,5 @@
module OmniAuth
module Concur
VERSION = "0.0.1"
VERSION = "0.0.2"

Choose a reason for hiding this comment

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

I appreciate that my rambling is being remembered 😄

Copy link

@dezkowalski dezkowalski left a comment

Choose a reason for hiding this comment

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

LGTM. Your documentation / commit messages made this super easy to follow & understand why each change is happening. This devcontainer approach seems really interesting!

@ncperry ncperry changed the title Ie-517 Ruby 3.0.6 Compatibility IE-517 Ruby 3.0.6 Compatibility Feb 5, 2024
@ncperry ncperry merged commit 93220d3 into master Feb 5, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants