Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove XML Parser from ActionDispatch #9328

Merged
merged 1 commit into from Feb 20, 2013

Conversation

sikachu
Copy link
Member

@sikachu sikachu commented Feb 19, 2013

As per our discussion, this is going to be extracted out to a gem. If you want an ability to parse XML parameters, please install actiondispatch-xml_params_parser actionpack-xml_parser gem.

@steveklabnik
Copy link
Member

👍

1 similar comment
@haileys
Copy link
Contributor

haileys commented Feb 19, 2013

👍

@homakov
Copy link
Contributor

homakov commented Feb 19, 2013

I cannot see a merge button? Anyone, if you see the "merge" button, click, ASAP

@sikachu
Copy link
Member Author

sikachu commented Feb 19, 2013

Holding this one off right now to discuss the right gem name for this. Potential candidates are:

  1. actiondispatch-xml_params_parser
  2. action_dispatch-xml_params_parser
  3. actionpack-xml_params_parser

I like 2 myself, since it follows Ruby require convention.

@steveklabnik
Copy link
Member

My vote is blue...errr, 3, because it's extending the actionpack gem.

@carlosantoniodasilva
Copy link
Member

3 👍

@mdespuits
Copy link

Yeah, 3 👍

@xiaods
Copy link
Contributor

xiaods commented Feb 19, 2013

yeah. 3 👍

@sikachu
Copy link
Member Author

sikachu commented Feb 20, 2013

Vote closed, and now I'm having a facepalm moment.

You could say that it's extending actionpack, but then you also have to think that actionpack is actually consist of abstract_controller, action_controller, action_dispatch, and action_view. actionpack itself is a container ...

I'm writing a documentation for ActionDispatch::XmlParamsParser now, and it doesn't make sense if the gem is going to be named actionpack-xml_params_parser. Also, if I renamed the namespace to ActionPack::XmlParamsParser, it started to feel weird as well.

Can I use my veto power and do 2?

@frodsan
Copy link
Contributor

frodsan commented Feb 20, 2013

@sikachu
Copy link
Member Author

sikachu commented Feb 20, 2013

I see. So I guess it should be in actionpack prefix. nvm then.

@xiaods
Copy link
Contributor

xiaods commented Feb 20, 2013

@sikachu how about your choice?

@sikachu
Copy link
Member Author

sikachu commented Feb 20, 2013

We've decided that we'll go with actionpack-xml_parser. A bit shorter, while keeping the gem within actionpack- prefix.

@sikachu
Copy link
Member Author

sikachu commented Feb 20, 2013

Code update mentioning the new library name. The code also has been extracted to https://github.com/rails/actionpack-xml_parser

@nicolai86
Copy link

👍

@rafaelfranca
Copy link
Member

Is something missing?

If you want an ability to parse XML parameters, please install
`actionpack-xml_parser` gem.
@sikachu
Copy link
Member Author

sikachu commented Feb 20, 2013

I think this is ready, and the green button is appearing again ... click it!

guilleiguaran added a commit that referenced this pull request Feb 20, 2013
Remove XML Parser from ActionDispatch
@guilleiguaran guilleiguaran merged commit ebae71a into rails:master Feb 20, 2013
@rafaelfranca
Copy link
Member

Thank you @guilleiguaran @sikachu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet