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

Added support for Rails 5.0, 5.1 and 5.2 #143

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open

Conversation

parse
Copy link

@parse parse commented Feb 26, 2016

Made a few changes to make RocketPants support Rails 5.

@parse
Copy link
Author

parse commented Mar 6, 2017

Any plans to get this into master @Sutto ?

@Sutto
Copy link
Owner

Sutto commented Mar 15, 2017

Hey @parse, sorry I missed this - I've been ignoring this repository for silly reasons.

Does this break backwards compat? Travis is broken so I can't check there, but will try to fix Travis ASAP to make sure I know.

If not broken, I'll merge it in asap and release.

Thank you for contributing - I appreciate it and really regret being pretty bad at running this. I'm looking to add some more people to help manage it.

@parse
Copy link
Author

parse commented Mar 15, 2017

I have not tested it on rails 4 sorry. I've been using it on my rails 5 setup for about a year now so it's been pretty stable.

Awesome that you are coming back to this now!

@parse parse changed the title Added support for Rails 5 [beta 3] Added support for Rails 5 Mar 15, 2017
@sevenseacat
Copy link

I've been using this fork while upgrading to Rails 5, and it throws one deprecation warning when running tests (over and over again):

DEPRECATION WARNING: Using positional arguments in functional tests has been deprecated,
in favor of keyword arguments, and will be removed in Rails 5.1.

Deprecated style:
get :show, { id: 1 }, nil, { notice: "This is a flash message" }

New keyword style:
get :show, params: { id: 1 }, flash: { notice: "This is a flash message" },
  session: nil # Can safely be omitted.
 (called from process at /Users/becky/.gem/ruby/2.3.3/bundler/gems/rocket_pants-a87933747c0f/lib/rocket_pants/test_helper.rb:107)

This is being thrown in non-Rocket-Pants-related controller specs, when the request in question doesn't actually have any parameters, ie. get :new.

If I add empty params (eg. get :new, params: {}) or remove the RocketPants test helper, the deprecation notice goes away.

@Sutto
Copy link
Owner

Sutto commented Mar 28, 2017

Presumably something having issues with our implementation of the test helpers (get etc) in that case?

@sevenseacat
Copy link

Yeah - the process method in the test helper instantiates the params argument to add the version param if necessary. There's already clauses in there for Rails 3 and Rails 4; a separate one might need to be added for Rails 5.

@parse
Copy link
Author

parse commented Mar 29, 2017 via email

@parse
Copy link
Author

parse commented Apr 12, 2017

@sevenseacat The tests have now been fixed for Rails 5. Enjoy!

jordanmichaelrushing added a commit to jordanmichaelrushing/rocket_pants that referenced this pull request Apr 12, 2017
Copying changes over from this PR on the main repo for Rails 5 support: Sutto#143
@iujlaki
Copy link

iujlaki commented Apr 18, 2017

👍

@MatayoshiMariano
Copy link

Will this PR be merged?

@avitus
Copy link

avitus commented Jun 14, 2017

Would love to get this merged in. After looking through all the various replacement options now that I've switched to Rails 5.1, I still massively prefer this gem.

@Sutto
Copy link
Owner

Sutto commented Jun 15, 2017

Does this still work for Rails 4 and 3? 3 is less of a concern, but Rails 4 should be maintained.

If w can make sure it works on that and doesn't break other stuff, I'm happy to merge it in.

@MatayoshiMariano
Copy link

@Sutto how can we asure that? Fixing the tests?

@parse
Copy link
Author

parse commented Jun 16, 2017

@avitus I submitted a PR here: Sutto/api_smith#12

@parse parse changed the title Added support for Rails 5 Added support for Rails 5.0 and 5.1 Jun 16, 2017
@parse
Copy link
Author

parse commented Aug 4, 2017

Support for Rails 5.1 was kindly added by @MatayoshiMariano

@parse parse changed the title Added support for Rails 5.0 and 5.1 Added support for Rails 5.0, 5.1 and 5.2 Apr 17, 2018
@parse
Copy link
Author

parse commented Apr 17, 2018

This now works for Rails 5.2 as well. @Zhong-z kindly added support for Bugsnag v6.

@Sutto Any chance we can get this merged and cut a new major version perhaps to avoid breaking support for Rails 4. (FYI, I am not using Rails 4, but I haven't heard about any issues on it for people using this fork)

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.

7 participants