hanami / router

Ruby/Rack HTTP router
http://hanamirb.org
MIT License
362 stars 92 forks source link

Update custom parser example in README.md #146

Closed beauby closed 7 years ago

beauby commented 7 years ago

The example custom body parser defines instance methods (#mime_types,#parse) whereas hanami-router expects class methods.

davydovanton commented 7 years ago

Are you sure about that? I checked code and as I can see Parsing expects instance methods

https://github.com/hanami/router/blob/master/lib/hanami/routing/parsing/parser.rb#L38-L45

jodosha commented 7 years ago

@beauby Thanks for this PR.

A parser should have instance methods, not class methods. Please have a look at this: https://github.com/hanami/router/blob/master/lib/hanami/routing/parsing/json_parser.rb#L11

Is there anything that may lead to think to class methods? Thanks 😄

beauby commented 7 years ago

@jodosha Actually, it had to do with the fact that there is no example for how to register the body parser (at least not in that section), which lead me to register it as body_parsers MyParser, whereas I suppose it is expected to be registered as body_parser MyParser.new.

jodosha commented 7 years ago

@beauby I see, thank you for your reply.

Do you think we should somehow modify this example here? https://github.com/hanami/router#custom-parsers It mentions there to use an object and not a class, but still you were confused.

Long story short, it seems a communication problem or poor documentation.

Given this PR is not right, do you suggest any other area of intervention that would have helped you to retrieve the right information? Thanks 😃

beauby commented 7 years ago

@jodosha No I think the doc of hanami-router is fine. I'd suggest adding a line to http://hanamirb.org/guides/actions/parameters/ in the body parsers section to show a complete example of custom body parser creation and registration.

Closing this PR. Thanks!

jodosha commented 7 years ago

@beauby Would you be so kind to craft a PR for that then? Thanks.

beauby commented 7 years ago

@jodosha https://github.com/hanami/hanami.github.io/pull/327