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

Rails 6 #785

Merged
merged 44 commits into from Feb 22, 2019
Merged

Rails 6 #785

merged 44 commits into from Feb 22, 2019

Conversation

ghost
Copy link

@ghost ghost commented Feb 20, 2019

Adapted from: https://github.com/plataformatec/devise/blob/master/lib/generators/active_record/devise_generator.rb#L86

I have also changed double quotes to single so that it wouldn't conflict with the previous PR.

@ghost
Copy link
Author

ghost commented Feb 20, 2019

All rbx builds were failing, so I removed it from .travis.yml

@ghost ghost changed the title Enable Rails 6 migrations Rails 6 Feb 21, 2019
@ghost
Copy link
Author

ghost commented Feb 21, 2019

There is a weird bug (heartcombo/devise#4630) in newer versions of Ruby (2.5+) that keeps breaking the build for older versions of Devise (< 4.4). See: https://travis-ci.org/scambra/devise_invitable/builds/496151257

I'd say to drop Devise 4.0/4.1 support, since these versions are not being maintained, afaik.

Note: Minimum supported Ruby version should be upped to 2.3.8 (which will EOL next month). See: https://www.ruby-lang.org/en/downloads/branches/

Note to self:

DEPRECATION WARNING: secrets.secret_token is deprecated in favor of secret_key_base and will be removed in Rails 6.0.

.travis.yml Outdated Show resolved Hide resolved
@scambra
Copy link
Owner

scambra commented Feb 21, 2019

You're right, devise 4.0 and 4.1 should be removed, can you change those gemfiles to devise 4.3 and 4.5, and change Gemfile to devise 4.6? Also, I see Gemfile.devise-4.4 and Gemfile.rails-4.2 are testing same versions, maybe all devise-x gemfiles should be changed to test agains rails 5.2, and just keep one Gemfile for rails 4.2 with latest devise version.

There is Gemfile for rails 5.2, but it isn't added to travis.yml, can you add it?

I see you added ruby 2.6 and then removed, you can add it and exclude gemfiles which won't be supported, in exclude block.

This lines can be removed from exclude:

    - rvm: 2.1.10
      gemfile: Gemfile

@ghost ghost mentioned this pull request Feb 21, 2019
@ghost
Copy link
Author

ghost commented Feb 21, 2019

Rails 4.2 is incompatible with this release, so I dropped the Gemfile.

@ghost
Copy link
Author

ghost commented Feb 21, 2019

Mongoid keeps failing on this test: https://github.com/scambra/devise_invitable/blob/master/test/models/invitable_test.rb#L167

@scambra, can you please take a look?

Version before / after it started breaking:

  • mongo - 2.5.1 / 2.7.0
  • mongoid - 5.2.1 / 7.0.2
  • bson - 4.3.0 / 4.4.2

Here is the error message:
Expected #<ActiveModel::Errors:0x0000000005a4baa0 @base=#<User _id: BSON::ObjectId('5c6ed8086905e844638d82b8'), active: "true", email: "valid@email.com", profile_id: "1", username: nil>, @messages={:active=>["is invalid"]}, @details={:active=>[{:error=>:invalid}]}> to be empty.

EDIT: It happens due to some weird BSON serialization: https://stackoverflow.com/questions/18657071/why-mongoid-doesnt-save-boolean-as-true-or-false

I removed boolean from invite_key test. It worked fine for active record though.

@ghost
Copy link
Author

ghost commented Feb 22, 2019

It's ready 🎉

This PR turned out to be bigger than I anticipated it to be. A lot of commits in this branch are pretty trivial or outright contradict each other (the whole jazz with versions). Don't forget to squash all commits, otherwise it will pollute commit history for no good reason.

https://help.github.com/en/articles/configuring-commit-squashing-for-pull-requests

@scambra scambra merged commit 8c313de into scambra:master Feb 22, 2019
@scambra
Copy link
Owner

scambra commented Feb 22, 2019

Great job, thanks

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

2 participants