Skip to content
This repository has been archived by the owner on Jul 9, 2024. It is now read-only.

Use render plain: instead of the deprecated render text: #85

Merged
merged 3 commits into from
Aug 9, 2018

Conversation

petervandoros
Copy link
Contributor

@petervandoros petervandoros commented Aug 8, 2018

Rails 5 deprecated render text: 'plain text'. Instead, render plain: 'plain text' should be used.

Considerations

  • I'll publish the new version after merging to master.

@petervandoros petervandoros requested review from zubin and shevaun August 8, 2018 10:43
Copy link
Contributor

@zubin zubin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Since it's open source, perhaps we should mention the minimum rails version (when render plain was introduced).

Copy link

@johnsyweb johnsyweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏆

@shevaun
Copy link

shevaun commented Aug 8, 2018

Yeah I think we should specify a dependency on Rails 5 for this version (https://github.com/envato/guide/blob/master/guide.gemspec#L17), since we are removing the handling for versions less than 5, otherwise +1

@zubin
Copy link
Contributor

zubin commented Aug 8, 2018

If we're dropping support for rails 4 then the Appraisals file should be updated, although I don't think we need to drop support just yet since render plain works in rails 4.

@petervandoros
Copy link
Contributor Author

@shevaun We don't need to remove Rails 4 yet as it still works with it (like @zubin already stated). You can see the build which runs the tests against Rails 4 and 5.

If you want me to remove support for Rails 4, just let me know and I can do that too.

@shevaun
Copy link

shevaun commented Aug 9, 2018

Oh ok, so the tests pass in Rails 4? I just assumed expect(response).not_to render_template(:show) didn't work since it was wrapped in a if Rails 5. If it does, I'm fine to leave it as is :)

update: I just looked at the build and saw it does in fact pass. Great!

@petervandoros petervandoros merged commit 117e1f4 into master Aug 9, 2018
@petervandoros petervandoros deleted the render-plain-not-text branch August 9, 2018 01:20
@petervandoros
Copy link
Contributor Author

Gem published to rubygems.org (https://rubygems.org/gems/guide)

$ gem push guide-0.4.1.gem
Enter your RubyGems.org credentials.
Don't have an account yet? Create one at https://rubygems.org/sign_up
   Email:   rubygems@envato.com
Password:

Signed in.
Pushing gem to https://rubygems.org...
Successfully registered gem: guide (0.4.1)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants