search

codeplant / simple-navigation

A ruby gem for creating navigations (with multiple levels) for your Rails, Sinatra or Padrino applications. Render your navigation as html list, link list or breadcrumbs.
http://simple-navigation-demo.codeplant.ch/
MIT License
885 stars 136 forks source link

Code cleaning #149

Closed simonc closed 10 years ago

simonc commented 10 years ago

I cleaned the code to respect the Ruby style-guides, some parts are still a bit weird though. Along the way, I changed some methods by splitting them in sub-private-methods but I kept the API intact.

andi commented 10 years ago

Ok thx a lot. I will release 3.12. without these changes since it will take some time to review. Not that I don't trust you, but I'm interested in the changes. As I said, 3.13 will be released in about 2-3 weeks.

On Tue, Feb 25, 2014 at 4:44 PM, Simon Courtois notifications@github.comwrote:

I cleaned the code to respect the Ruby style-guides, some parts are still a bit weird though. Along the way, I changed some methods by splitting them in

sub-private-methods but I kept the API intact.

You can merge this Pull Request by running

git pull https://github.com/simonc/simple-navigation code-cleaning

Or view, comment on, or merge it at:

https://github.com/codeplant/simple-navigation/pull/149 Commit Summary

  • Cleaning up the config generator code
  • Cleaning up the init code
  • Cleaning up the simple-navigation code
  • Cleaning up the simple_navigation code
  • Cleaning up the Base adapter code
  • Cleaning up the Nanoc adapter code
  • Cleaning up the Padrino adapter code
  • Cleaning up the Rails adapter code
  • Cleaning up the Sinatra adapter code
  • Cleaning up the Configuration code
  • Cleaning up the Item code
  • Cleaning up the ItemAdapter code
  • Cleaning up the ItemContainer code
  • Cleaning up the ItemProvider code
  • Cleaning up the RailsControllerMethods code
  • Cleaning up the Helpers code
  • Cleaning up the Base renderer code
  • Cleaning up the Breadcrumbs renderer code
  • Cleaning up the Json renderer code
  • Cleaning up the Links renderer code
  • Cleaning up the List renderer code
  • Cleaning up the Text renderer code

File Changes

  • M generators/navigation_config/navigation_config_generator.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-0(6)
  • M generators/navigation_config/templates/config/navigation.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-1(19)
  • M init.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-2(2)
  • M lib/generators/navigation_config/navigation_config_generator.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-3(17)
  • M lib/simple-navigation.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-4(2)
  • M lib/simple_navigation.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-5(158)
  • M lib/simple_navigation/adapters/base.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-6(32)
  • M lib/simple_navigation/adapters/nanoc.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-7(13)
  • M lib/simple_navigation/adapters/padrino.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-8(12)
  • M lib/simple_navigation/adapters/rails.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-9(91)
  • M lib/simple_navigation/adapters/sinatra.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-10(31)
  • M lib/simple_navigation/core/configuration.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-11(107)
  • M lib/simple_navigation/core/item.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-12(164)
  • M lib/simple_navigation/core/item_adapter.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-13(31)
  • M lib/simple_navigation/core/item_container.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-14(159)
  • M lib/simple_navigation/core/items_provider.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-15(22)
  • M lib/simple_navigation/rails_controller_methods.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-16(176)
  • M lib/simple_navigation/rendering/helpers.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-17(198)
  • M lib/simple_navigation/rendering/renderer/base.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-18(55)
  • M lib/simple_navigation/rendering/renderer/breadcrumbs.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-19(45)
  • M lib/simple_navigation/rendering/renderer/json.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-20(24)
  • M lib/simple_navigation/rendering/renderer/links.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-21(31)
  • M lib/simple_navigation/rendering/renderer/list.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-22(43)
  • M lib/simple_navigation/rendering/renderer/text.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-23(19)
  • M spec/lib/simple_navigation/core/item_adapter_spec.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-24(2)
  • M spec/lib/simple_navigation/core/item_container_spec.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-25(186)
  • M spec/lib/simple_navigation_spec.rbhttps://github.com/codeplant/simple-navigation/pull/149/files#diff-26(21)

Patch Links:

— Reply to this email directly or view it on GitHubhttps://github.com/codeplant/simple-navigation/pull/149 .

simonc commented 10 years ago

No problem :)

andi commented 10 years ago

Is it appropriate to drop support for ruby 1.8.7? There might be some crazy people around that are still using it. What do you think?

simonc commented 10 years ago

Ruby is dropping 1.8, Rails is dropping 1.8. I think it's pretty safe :)

andi commented 10 years ago

lol :-) true...

On Tue, Feb 25, 2014 at 5:18 PM, Simon Courtois notifications@github.comwrote:

Ruby is dropping 1.8, Rails is dropping 1.8. I think it's pretty safe :)

— Reply to this email directly or view it on GitHubhttps://github.com/codeplant/simple-navigation/pull/149#issuecomment-36025028 .