Merged
Conversation
`ActiveRecord::MigrationContext` now has a `schema_migration` attribute. Ref: https://github.com/rails/rails/pull/36439/files#diff-8d3c44120f7b67ff79e2fbe6a40d0ad6R1018
Before Rails 6 RC2, the `ActionDispatch::Response#content_type` method would return only the media part of the `Content-Type` header, without any other parts. Now the `#content_type` method returns the entire header - as it is - and `#media_type` should be used instead to get the previous behavior. Ref: - rails/rails#36034 - rails/rails#36854
Render file will need the full path in order to avoid security breaches. In this particular case, there's no need to use render file, it's ok to use render template. Ref: rails/rails#35688
lucasnar
approved these changes
Aug 7, 2019
lucasnar
left a comment
There was a problem hiding this comment.
I'm not very familiar with Devise. Not sure if my comments make sense. 😅
|
|
||
| # Remove this check once Rails 5.0 support is removed. | ||
| if Devise::Test.rails52_and_up? | ||
| if Devise::Test.rails52_and_up? && !Devise::Test.rails6? |
There was a problem hiding this comment.
Probably something like # Remove the first check once Rails 5.0 support is removed. now? Do you know if Rails 6 is always going to require this? (It might make sense to update the docs to consider it too.)
Member
Author
There was a problem hiding this comment.
We'll still need the rails6? check once we remove support for Rails 5 because here we need to set this config for Rails 5.2. When Rails 6 is the only supported version, then we can remove this code altogether.
| module Test | ||
| # Detection for minor differences between Rails 4 and 5, 5.1, and 5.2 in tests. | ||
|
|
||
| def self.rails6? |
There was a problem hiding this comment.
Should we update this module's docs as well?
Member
Author
|
@lucasnar Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
ActiveRecord::MigrationContextclass now has aschema_migrationattribute.Reference: https://github.com/rails/rails/pull/36439/files#diff-8d3c44120f7b67ff79e2fbe6a40d0ad6R1018
This pull request also includes some changes to avoid deprecation warnings: 3273f9e, 77cb394 and 69534c6