This project is archived and is in readonly mode.
Delete useless constructor in XML Serializers
Reported by Jakub Okoński | September 3rd, 2010 @ 04:57 PM
I think this constructor is useless, as to_xml triggers the
second constructor, which is working fine.
Jose commited at 29 April with an error in this not used
constructor and it has been undetected for few months now.
http://github.com/rails/rails/commit/2e9af3638d950ef840e1287f99e323...
Also, andreas's tests of new patch that fixed this undetected error are wrong, as they use to_xml which triggers other constructor (not that one he fixed).
Comments and changes to this ticket
-
Jakub Okoński September 3rd, 2010 @ 05:00 PM
Having some problems with lighthouse attachments, here is a patch:
http://gist.github.com/564092 -
José Valim September 3rd, 2010 @ 05:44 PM
- Importance changed from “” to “Low”
Awesome! I will apply it later! Thanks!
-
José Valim September 18th, 2010 @ 08:19 PM
Why do we need to delete the test? Does it pass or fail after your changes?
-
Jakub Okoński September 19th, 2010 @ 04:19 PM
The test is wrong, because it triggers working construcor of XML Serializer. This one, that I call 'useless' is not triggered by any other function, so why keeping it?
-
José Valim September 19th, 2010 @ 04:33 PM
In any case, Andrea have found an issue. Shouldn't we have a test for the issue he found?
-
Jakub Okoński September 19th, 2010 @ 07:02 PM
This test of his is wrong, because it triggers not this function that he fixed, but the other.
Post.where(:id => 12345).first.to_xml triggers second constructor(def initialize), not the one he fixed.
I'll try to fix his test and post a patch.
-
Jakub Okoński September 19th, 2010 @ 07:16 PM
Please test this one, as my bundler installation is broken someone.
-
Jakub Okoński September 19th, 2010 @ 07:22 PM
Got myself really confused by all this... try this one:
-
José Valim September 24th, 2010 @ 11:45 AM
- State changed from “new” to “incomplete”
Applying your patch that removes the constructor causes several ActiveModel tests to fail.
-
Santiago Pastorino February 2nd, 2011 @ 04:31 PM
- State changed from “incomplete” to “open”
This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.
Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.
-
Santiago Pastorino February 2nd, 2011 @ 04:31 PM
- State changed from “open” to “stale”
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
Jakub Okoński
Jeremy Kemper
José Valim