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

Add app.rb and helpers.rb back for backward compatibility #51839

Merged
merged 1 commit into from
May 24, 2024

Conversation

st0012
Copy link
Contributor

@st0012 st0012 commented May 15, 2024

Motivation / Background

Those files were removed in #51760, but gems like console1984 depend on these files for legacy Rails console command extensions. So keeping files around is required for backward-compatibility.

Detail

The files were added back and just require rails/console/methods.rb instead with messages like:

DEPRECATION WARNING: `rails/console/app.rb` has been deprecated and will be removed in Rails 8.0.
Please require `rails/console/methods.rb` instead.
 (called from <main> at /path_to_rails/railties/lib/rails/console/app.rb:3)

DEPRECATION WARNING: `rails/console/helpers.rb` has been deprecated and will be removed in Rails 8.0.
Please require `rails/console/methods.rb` instead.
 (called from <main> at /path_to_rails/railties/lib/rails/console/helpers.rb:3)

Additional information

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

Copy link
Member

@carlosantoniodasilva carlosantoniodasilva left a comment

Choose a reason for hiding this comment

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

I think we need to point this to 7-2-stable now that it's been created, so we keep the files there (where console methods are deprecated) but remove from main (where that's likely to be removed anyway)... wdyt?

@st0012 st0012 changed the base branch from main to 7-2-stable May 15, 2024 15:04
@st0012 st0012 requested a review from rafaelfranca as a code owner May 15, 2024 15:04
@st0012
Copy link
Contributor Author

st0012 commented May 15, 2024

@carlosantoniodasilva 👍 updated

@@ -0,0 +1,3 @@
# frozen_string_literal: true

require "rails/console/methods"
Copy link
Member

Choose a reason for hiding this comment

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

Should we also add a deprecation warning here so there's a notice before removal in 8.0?

Choose a reason for hiding this comment

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

Yes, let's add a deprecation to the individual files as well, otherwise someone importing it might not realize the files themselves are also going away.

Copy link
Contributor Author

@st0012 st0012 May 23, 2024

Choose a reason for hiding this comment

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

I actually committed it last week but somehow lost it when switching branches 😅
All updated now. PR description has an example message.

@ghiculescu
Copy link
Member

ghiculescu commented May 15, 2024

I think we need to point this to 7-2-stable now that it's been created, so we keep the files there (where console methods are deprecated) but remove from main (where that's likely to be removed anyway)... wdyt?

Please merge it on main too 🙏 The issue currently occurs when running Edge Rails. So if this is not fixed on main, then when the next Rails release goes out, it will still be a breaking change for downstream gems.

@rafaelfranca
Copy link
Member

Let's deprecate and merge on main as well. We will remove the deprecation later.

Those files were removed in rails#51760, but gems like `console1984` depend
on these files for legacy Rails console command extensions.
So keeping files around is required for backward-compatibility.
@carlosantoniodasilva carlosantoniodasilva merged commit 46cd9f7 into rails:main May 24, 2024
3 checks passed
carlosantoniodasilva added a commit that referenced this pull request May 24, 2024
Add app.rb and helpers.rb back for backward compatibility
@st0012 st0012 deleted the keep-deprecated-files branch May 26, 2024 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants