search

weppos / breadcrumbs_on_rails

A simple Ruby on Rails plugin for creating and managing a breadcrumb navigation.
https://simonecarletti.com/code/breadcrumbs-on-rails
MIT License
944 stars 188 forks source link

before_filter is deprecated in rails 5.0 #80

Closed ochko closed 8 years ago

ochko commented 8 years ago

and will be removed in 5.1, use before_action

simonharrer commented 8 years ago

:+1:

mibamur commented 8 years ago

could someone check it?

addicted2sounds commented 8 years ago

Will this PR be fixed?

ochko commented 8 years ago

Hmmm, the build was failed at 'bundle install' stage for ruby 2.2.0. It is passing on my local.

weppos commented 8 years ago

The failures are not directly connected, I'll see if I can update the travis config. However, this PR doesn't include any test for the change, hence I'm not going to merge it unless it is updated with some tests.

I'm sorry, but I don't accept patches that changes code without corresponding unit tests.

SirNerdBear commented 8 years ago

A unit test for a single alias method call? That's a ridiculous standard. All you're really doing is forcing everyone who has adapted rails 5, to be using our own branch of this gem. You're seriously missing the point and purpose of unit testing. You have a gem that hasn't been updated in a year, you should seriously consider passing maintenance to someone else who has more time.

weppos commented 8 years ago

A unit test for a single alias method call? That's a ridiculous standard. You're seriously missing the point and purpose of unit testing.

The PR pretends to introduce compatibility with Rails 5. So the unit test has to be there to guarantee that, in fact, the code is compatible with Rails 5.

ochko commented 8 years ago

ruby 2.2 build is removed for rails 4, and added a build for rails 5.

ochko commented 8 years ago

README.md says:

## Requirements

- Rails 3 or Rails 4

So I would prefer adding test for Rails 3.

weppos commented 8 years ago

@ochko I'm going to kill support for Rails 3 in the next release.

ochko commented 8 years ago

I see. Shall I remove rails 3 support in this pull request?

weppos commented 8 years ago

No, I'll take care of that in another branch.

On Tue, Jul 26, 2016 at 3:29 AM, Ochko notifications@github.com wrote:

I see. Shall I remove rails 3 support in this pull request?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/weppos/breadcrumbs_on_rails/pull/80#issuecomment-235136568, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAVC1S3xuSsiUPhT-Xc6eAoYMJRhgntks5qZWLpgaJpZM4G-nxW .

Simone Carletti Passionate programmer and dive instructor

https://simonecarletti.com/ Twitter: @weppos https://twitter.com/weppos

weppos commented 8 years ago

Thanks for the cooperation and follow ups @ochko, I really appreciate it.