search

takafumir / rails_amp

RailsAmp is a Ruby on Rails plugin for AMP(Accelerated Mobile Pages).
MIT License
65 stars 26 forks source link

add: trailing slash option to rails_amp_amphtml_link_tag #3

Closed yamadagenki closed 7 years ago

yamadagenki commented 7 years ago

hi, I add trailing slash option to rails_amp_amphtml_link_tag.

The usage is written in README.md.

Please check it.

Thanks,

takafumir commented 7 years ago

Hi, thank you for your contribution. I'm so sorry but I can't merge your pull request, because I don't think your use case is so common. You can override RailsAmp's rails_amp_amphtml_link_tag helper or create other helper in your own library (e.g. rails_project/lib/rails_amp_extension). I want to focus the core functions and the common use cases in this RailsAmp gem. I would appriciate your understanding. 🙏

yamadagenki commented 7 years ago

😢

yamadagenki commented 7 years ago

I rethink about this problem.

"slash_trailing" option is included in Ruby On Rails default routing options. I think this option is common usage, and rails_amp gem should support it.

https://github.com/rails/rails/blob/cfb1e4dfd8813d3d5c75a15a750b3c53eebdea65/actionpack/lib/action_dispatch/routing/route_set.rb#L768

And from SEO point of view, I think it is an important option. https://webmasters.googleblog.com/2010/04/to-slash-or-not-to-slash.html

So please, reconsider about this pull request.

Thanks,