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

[ci skip] Remove comments about Rails 3.1 #20113

Merged
merged 1 commit into from
May 14, 2015

Conversation

claudiob
Copy link
Member

Stems from #20105 (comment) where @senny said:

From my point of view, all the docs (guides, API) are version bound.
They should describe that version and continue to be available when newer versions are released.
The cross referencing can be done by the interested user.

@senny, merge/reject as you wish… I don't have a strict preference about this.

# contains the version numbers of all the migrations applied.
#
# As a result, it is now possible to add migration files that are numbered
# It is possible to add migration files that are numbered
Copy link
Member

Choose a reason for hiding this comment

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

This sounds weird as an opening sentence under the sub-heading.

However, I think this whole paragraph is really still describing how the 2.1+ solution fixes a shortcoming in the old approach... so it can probably go too.

@matthewd
Copy link
Member

👍

@claudiob
Copy link
Member Author

@matthewd Removed the whole paragraph. On a separate note, as I was reading that file, I saw this:

# If you'd prefer to use numeric prefixes, you can turn timestamped migrations
# off by setting:
#
#    config.active_record.timestamped_migrations = false
#
# In application.rb.

Is this an actual configuration that we want to maintain in Rails 5, 6, 7, or was it just a temporary patch for people upgrading to Rails 3, and can be removed in Rails 5?

@senny
Copy link
Member

senny commented May 11, 2015

@claudiob can you make sure that the upgrading guide covers topics relevant for upgrading?

Other than that I'm 👍, let's run it by @fxn to be sure about the direction.

@@ -8,8 +8,7 @@ module ActionController
# POST requests without having to specify any root elements.
#
# This functionality is enabled in +config/initializers/wrap_parameters.rb+
# and can be customized. If you are upgrading to \Rails 3.1, this file will
# need to be created for the functionality to be enabled.
# and can be customized.
Copy link
Member Author

Choose a reason for hiding this comment

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

Stems from rails#20105 (comment)
where @senny said:

> From my point of view, all the docs (guides, API) are version bound.
> They should describe that version and continue to be available when newer versions are released.
> The cross referencing can be done by the interested user.
# your plugin's +lib+ folder (similar to how we specify a +Railtie+):
# If you want a gem to behave as an engine, you have to specify an +Engine+
# for it somewhere inside your plugin's +lib+ folder (similar to how we
# specify a +Railtie+):
Copy link
Member Author

Choose a reason for hiding this comment

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

Our upgrade guide starts with "Upgrading from Rails 3.0 to Rails 3.1".
This is also probably so old, we don't need to describe it anymore.

@claudiob
Copy link
Member Author

@senny I think everything relevant is covered, check my comments above 🎀

@fxn
Copy link
Member

fxn commented May 14, 2015

👍 over here!

senny added a commit that referenced this pull request May 14, 2015
[ci skip] Remove comments about Rails 3.1
@senny senny merged commit 7f60bed into rails:master May 14, 2015
@senny
Copy link
Member

senny commented May 14, 2015

@claudiob @fxn thanks guys 💛

@claudiob claudiob deleted the remove-rails31-refs branch May 14, 2015 19:55
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.

None yet

4 participants