Skip to content

Add support for Ruby 3.4 #38

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.

Sign up for GitHub

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

Jump to bottom
Merged
thatguyinabeanie merged 2 commits into from
Jan 9, 2025
Merged

Add support for Ruby 3.4 #38

thatguyinabeanie merged 2 commits into from
Jan 9, 2025

Conversation

@scudco
Copy link
Contributor

@scudco scudco commented Jan 7, 2025

No description provided.

@scudco scudco marked this pull request as ready for review January 9, 2025 16:33
@scudco
Copy link
Contributor Author

scudco commented Jan 9, 2025

@mattyb can someone at Civis approve the workflow run on this PR?

mikesaelim
mikesaelim reviewed Jan 9, 2025
View reviewed changes
Copy link
Contributor

@mikesaelim mikesaelim left a comment

Choose a reason for hiding this comment

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

Unfortunately, my PR #37 got merged in ahead of yours, and it's created some merge conflicts with your PR. Could you rebase your PR on top of main? Sorry about this.

Gemfile Outdated
gem 'rake', '~> 13.0'
gem 'rspec', '~> 3.9'
gem 'rubocop', '~> 1.69.2'
gem 'timecop', '~> 0.9.1'
Copy link
Contributor

@mikesaelim mikesaelim Jan 9, 2025

Choose a reason for hiding this comment

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

I was going to ask if these could be put back in the gemspec file, but I guess this is because of a new Rubocop cop? https://docs.rubocop.org/rubocop/cops_gemspec.html#gemspecdevelopmentdependencies

Copy link
Contributor Author

@scudco scudco Jan 9, 2025

Choose a reason for hiding this comment

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

yeah and same with the MFA requirement added to the gemspec and grep_v stuff.

@thatguyinabeanie thatguyinabeanie self-requested a review January 9, 2025 20:38
@thatguyinabeanie
Copy link
Contributor

thatguyinabeanie commented Jan 9, 2025 •
edited
Loading

@mattyb can someone at Civis approve the workflow run on this PR?

@scudco ran your workflow for ya.

thatguyinabeanie
thatguyinabeanie approved these changes Jan 9, 2025
View reviewed changes
Copy link
Contributor

@thatguyinabeanie thatguyinabeanie left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LggTM. thank you so much for taking this on <3

@scudco i will cut a release very soon.

@thatguyinabeanie thatguyinabeanie merged commit ae53292 into civisanalytics:main Jan 9, 2025
4 checks passed
@thatguyinabeanie thatguyinabeanie mentioned this pull request Jan 9, 2025
Merged
@scudco scudco deleted the ruby-3.4 branch January 18, 2025 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thatguyinabeanie thatguyinabeanie thatguyinabeanie approved these changes

+1 more reviewer

@mikesaelim mikesaelim mikesaelim left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@scudco @thatguyinabeanie @mikesaelim