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.

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

Rails 72 fix deprecation warn #948

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Rails 72 fix deprecation warn #948

wants to merge 4 commits into from
+3 −3

Conversation

Migoo
Copy link

@Migoo Migoo commented Dec 19, 2024

Fix deprecation method ActiveSupport::Deprecation.warn. Replaced with ActiveSupport::Deprecation.new.warn

ekohl
ekohl requested changes Dec 19, 2024
View reviewed changes
mathieujobin
mathieujobin requested changes Jan 3, 2025
View reviewed changes
Copy link
Collaborator

@mathieujobin mathieujobin left a comment

Choose a reason for hiding this comment

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

I agree with @ekohl 's comment.

@Migoo @ekohl
Update lib/apipie/application.rb
65e2cda 
Co-authored-by: Ewoud Kohl van Wijngaarden <[email protected]>
@Migoo Migoo requested review from ekohl and mathieujobin January 6, 2025 14:30
@ekohl
Copy link
Member

ekohl commented Jan 6, 2025

@Migoo the comment was applicable to to all lines changed and I just didn't want to repeat myself. Could you please apply it everywhere consistently?

@Migoo
Copy link
Author

Migoo commented Jan 7, 2025

@Migoo the comment was applicable to to all lines changed and I just didn't want to repeat myself. Could you please apply it everywhere consistently?

My bad ! I just push for all deprecation

ekohl
ekohl approved these changes Jan 7, 2025
View reviewed changes
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Code looks good to me so I just approved the tests to see if CI agrees.

@mathieujobin do you agree this is now good to go? I also think we should squash the commits on merge.

@ekohl
Copy link
Member

ekohl commented Jan 7, 2025

Looking at the tests, I see we're not testing on Rails 7.2 so perhaps it should also be added:

apipie-rails/.github/workflows/build.yml

Line 11 in 62b24b6

rails: ["7.1", "7.0", "6.1", "6.0"]

We can also ignore the Ruby 3.4 failure in this PR, but we should address that as well as change it to Ruby 3.4 final now that's out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ekohl ekohl ekohl approved these changes

@mathieujobin mathieujobin Awaiting requested review from mathieujobin

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
@Migoo @ekohl @mathieujobin