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

Fix name collision renaming breadcrumbs to breadcrumbs_on_rails #65

Closed ngelx closed 4 years ago

ngelx commented 10 years ago

This PR is based on the @justame changes, adding the suggested changes made by @weppos.

etiennechabert commented 9 years ago

What are we waiting for to merge this branch if it's passing tests ?

weppos commented 9 years ago

Tests are a way to (possibly) validate the correct behavior of a change, they don't necessarily mean the change makes sense.

In this case, the changes applies in this patch will breack backward-compatibility, therefore I can't merge and release them as part of a bugfix or mini release. They require a major release change.

I need to take the time to review the changes, and work on the release.

etiennechabert commented 9 years ago

Your analysis is correct, but this issue is quite annoying because lot of the people are using bootstrap. Are you aware that the solution on the readme for bootstrap users isn't working (for me at least). Actually I am not able to found a way to have a proper rendering without removing bootstrap.

weppos commented 9 years ago

Your analysis is correct, but this issue is quite annoying because lot of the people are using bootstrap.

I'm sorry if this annoys you, but my contribution to open source is proportional to my free time which at the time being is close to 0. Feel free to use a different library if this one doesn't fit your need, or to fork the project and model it against your desire.

As a maintainer, my goal is to make sure that the library fits everyone's need and doesn't suddenly break on a release.

etiennechabert commented 9 years ago

I am not complaining at all about your work on this project, you did a great job for the OpenSource community. But there is a known issue with bootstrap and for good reason contributors decided to didn't patch it for the moment, you may just add a warning on the readme. Actually there is a section for bootstrap users with a not working fix.

trappist commented 7 years ago

+1, I've changed gem source to ngelx:master for now to solve my problem. Not necessarily advocating merge, I'm not familiar with the backward compat issues.

CodingAnarchy commented 6 years ago

+1, this is a problem for me as well, and ngelx:master doesn't support Rails 5, so this needs to be resolved for us to move forward.