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

Update rolify to 5.1.0 #1299

Merged
merged 1 commit into from
Mar 1, 2017
Merged

Update rolify to 5.1.0 #1299

merged 1 commit into from
Mar 1, 2017

Conversation

shlok007
Copy link
Member

@differentreality
Copy link
Contributor

I think this should be included in #1087

Also @shlok007 you have another 6 open PRs, at least 3 of them have recent comments and need to be updated. I would strongly suggest you first take care of your existing PRs, and work on new stuff after they are merged.

@shlok007
Copy link
Member Author

shlok007 commented Feb 17, 2017

I think this should be included in #1087

The reason I am updating dependencies to their rails 5 compatible version separately is because some gems ( e.g. devise see changelog) have breaking changes with their rails 5 compatible version. Updating rails version and all dependencies, fixing their breaking changes in a single PR would be a little hard to review as well.

Also, not all gems follow semantic versioning or have maintained a perfect changelog. This could help us find out which gems are no longer maintained or doesn't have a compatible version with rails 5 yet ( eg. rails-observers more details here ) and we can look for alternatives before proceeding with #1087 .

Also @shlok007 you have another 6 open PRs, at least 3 of them have recent comments and need to be updated. I would strongly suggest you first take care of your existing PRs, and work on new stuff after they are merged.

Thank you! I strongly agree, was just trying to cross off a few minor things from the todo list. I'll keep this in mind. 🙂

@differentreality
Copy link
Contributor

I'll merge this, but let's open separate PRs for updating gems only if needed - I agree that for devise there are some more essential changes to be made, so opening a new PR just for that might make sense (depending on how many other gems have the same problem). Other than that, I would leave gem updating for the overall Rails 5 update.

@differentreality differentreality merged commit 851a5e3 into openSUSE:master Mar 1, 2017
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.

2 participants