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

Expand CI Matrix #233

Merged
merged 3 commits into from
Jan 7, 2025
Merged

Conversation

btalayeminaei
Copy link
Contributor

@btalayeminaei btalayeminaei commented Jan 5, 2025

  • Add Ruby 3.3 to CI matrix
  • Add Ruby 3.4 to CI matrix
  • Add Active Support 8 to CI matrix

CI is green on my fork btalayeminaei#1

Comment on lines 20 to 24
- { ruby: "2.7", gemfile: "active_support_6", bundler: "1" }
- { ruby: "3.0", gemfile: "active_support_6", bundler: "1" }
- { ruby: "3.1", gemfile: "active_support_6", bundler: "1" }
- { ruby: "3.2", gemfile: "active_support_6", bundler: "1" }
- { ruby: "3.3", gemfile: "active_support_6", bundler: "1" }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Active Support 6 relies on libraries (e.g. base64, bigdecimal, etc.) that are no longer part of the default gems in Ruby 3.4. Since business_time does not appear to rely on these libraries, I did not update the gemspec and chose to test Ruby 3.4 only with Active Support 7+. Please let me know if I should go a different route. Thanks!

/versions/3.4.1/lib/ruby/gems/3.4.0/gems/activesupport-6.1.7.10/lib/active_support/xml_mini.rb:5: warning: bigdecimal was loaded from the standard library, but is not part of the default gems starting from Ruby 3.4.0.
You can add bigdecimal to your Gemfile or gemspec to silence this warning.
/Users/beshadtalayeminaei/.rbenv/versions/3.4.1/lib/ruby/3.4.0/bundled_gems.rb:82:in 'Kernel.require': cannot load such file -- bigdecimal (LoadError)

Copy link
Collaborator

@rmm5t rmm5t left a comment

Choose a reason for hiding this comment

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

Please see inline comments.

@@ -6,10 +6,9 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
ruby: [2.7, "3.0", 3.1, 3.2]
ruby: [2.7, "3.0", 3.1, 3.2, 3.3, 3.4]
Copy link
Collaborator

Choose a reason for hiding this comment

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

[SUGGESTION] Let's limit the core matrix to the supported ruby versions and a subset of the older ruby and activesupport version combinations to the include.

Suggested change
ruby: [2.7, "3.0", 3.1, 3.2, 3.3, 3.4]
ruby: ["3.2", "3.3", "3.4"]

Comment on lines 20 to 24
- { ruby: "2.7", gemfile: "active_support_6", bundler: "1" }
- { ruby: "3.0", gemfile: "active_support_6", bundler: "1" }
- { ruby: "3.1", gemfile: "active_support_6", bundler: "1" }
- { ruby: "3.2", gemfile: "active_support_6", bundler: "1" }
- { ruby: "3.3", gemfile: "active_support_6", bundler: "1" }
Copy link
Collaborator

Choose a reason for hiding this comment

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

[SUGGESTION]

Suggested change
- { ruby: "2.7", gemfile: "active_support_6", bundler: "1" }
- { ruby: "3.0", gemfile: "active_support_6", bundler: "1" }
- { ruby: "3.1", gemfile: "active_support_6", bundler: "1" }
- { ruby: "3.2", gemfile: "active_support_6", bundler: "1" }
- { ruby: "3.3", gemfile: "active_support_6", bundler: "1" }
- { ruby: "2.7", gemfile: "active_support_6" }
- { ruby: "3.0", gemfile: "active_support_7" }
- { ruby: "3.1", gemfile: "active_support_7" }

- active_support_6
- active_support_7
Copy link
Collaborator

Choose a reason for hiding this comment

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

[SUGGESTION] Let's also add an active_support_8 here with a new gemfile.

@btalayeminaei btalayeminaei force-pushed the expand-ruby-ci-matrix branch from 93bf21d to 6298e81 Compare January 6, 2025 22:59
@btalayeminaei btalayeminaei changed the title Expand CI Matrix for Ruby Expand CI Matrix Jan 6, 2025
@btalayeminaei btalayeminaei requested a review from rmm5t January 6, 2025 23:05
Comment on lines 21 to 24
- { ruby: "2.7", gemfile: "active_support_6", bundler: "1" }
- { ruby: "2.7", gemfile: "active_support_7", bundler: "1" }
- { ruby: "3.0", gemfile: "active_support_7", bundler: "1" }
- { ruby: "3.1", gemfile: "active_support_7", bundler: "1" }
Copy link
Collaborator

Choose a reason for hiding this comment

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

[PLEASE REMOVE] bundler: "1" from each of these. They can run under the latest (default) version of bundler for each version of ruby.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. Thanks

@btalayeminaei btalayeminaei force-pushed the expand-ruby-ci-matrix branch from ed5697b to e5a136f Compare January 6, 2025 23:48
@btalayeminaei btalayeminaei requested a review from rmm5t January 7, 2025 02:14
Copy link
Collaborator

@rmm5t rmm5t left a comment

Choose a reason for hiding this comment

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

Looks good!

@rmm5t rmm5t merged commit 9ab2474 into bokmann:develop Jan 7, 2025
16 checks passed
@btalayeminaei btalayeminaei deleted the expand-ruby-ci-matrix branch January 7, 2025 12:57
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