rocket_pants icon indicating copy to clipboard operation
rocket_pants copied to clipboard

Added support for Rails 5.0, 5.1 and 5.2

Open parse opened this issue 8 years ago • 16 comments

Made a few changes to make RocketPants support Rails 5.

parse avatar Feb 26 '16 16:02 parse

Any plans to get this into master @Sutto ?

parse avatar Mar 06 '17 15:03 parse

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.

Sutto avatar Mar 15 '17 02:03 Sutto

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 avatar Mar 15 '17 05:03 parse

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.

sevenseacat avatar Mar 28 '17 07:03 sevenseacat

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

Sutto avatar Mar 28 '17 20:03 Sutto

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.

sevenseacat avatar Mar 29 '17 01:03 sevenseacat

I'm aware of the deprecations being logged in 5.0. In 5.1rc1 they have been removed permanently so it completely fails tests.

Currently don't have time to dig into this, so if anyone is up for it, I would love to get a PR on it and try to land this into master.

parse avatar Mar 29 '17 05:03 parse

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

parse avatar Apr 12 '17 11:04 parse

👍

iujlaki avatar Apr 18 '17 08:04 iujlaki

Will this PR be merged?

MatayoshiMariano avatar Jun 09 '17 20:06 MatayoshiMariano

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.

avitus avatar Jun 14 '17 18:06 avitus

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.

Sutto avatar Jun 15 '17 03:06 Sutto

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

MatayoshiMariano avatar Jun 15 '17 17:06 MatayoshiMariano

@avitus I submitted a PR here: https://github.com/Sutto/api_smith/pull/12

parse avatar Jun 16 '17 14:06 parse

Support for Rails 5.1 was kindly added by @MatayoshiMariano

parse avatar Aug 04 '17 15:08 parse

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)

parse avatar Apr 17 '18 12:04 parse