Refactor #3
Refactor #3
Conversation
Reader#close hasn't worked for a while. #rewind is a way to run #each multiple times. #version and #encoding seem to be completely missing, hence excised from comments.
Documentation provides a warning, but it might be a useful flag in specs and simple scripts with known-good data.
Have tried to align the gem with best practices as outlined here: http://weblog.rubyonrails.org/2009/9/1/gem-packaging-best-practices
Apparently we use 'gem build' now. Added a little more verbiage to the gemspec to silence a warning.
Now you have to check whether code value is valid manually if you want to (using Code#valid?). Probably the nice behaviour would be to accept any value when reading an ONIX file, but throw an exception on invalid values when writing out. But how to detect? One option might be to override to_xml for code accessors to add the check... hmm.
The SimpleProduct/APAProduct wrappers are useful, but necessarily
incomplete. It's useful to have a way to add an "interpretive" layer
above ONIX::Product, like APAProduct does, but in a more ad hoc way.
These interpretation modules can be chunked in whatever way makes sense
("all title methods" or "all getter methods", etc). I think APAProduct
can then be built up from a simple library of modules, but that can wait
another day.
This is a bit insane, but ah well, halfway done. Everything should be complete up to PR.13 of 2.1. And ONIX 3 is completely different, of course. Maybe we do need a code generation tool. Hm.
Reader#close hasn't worked for a while. #rewind is a way to run #each multiple times. #version and #encoding seem to be completely missing, hence excised from comments.
Documentation provides a warning, but it might be a useful flag in specs and simple scripts with known-good data.
Have tried to align the gem with best practices as outlined here: http://weblog.rubyonrails.org/2009/9/1/gem-packaging-best-practices
Apparently we use 'gem build' now. Added a little more verbiage to the gemspec to silence a warning.
Now you have to check whether code value is valid manually if you want to (using Code#valid?). Probably the nice behaviour would be to accept any value when reading an ONIX file, but throw an exception on invalid values when writing out. But how to detect? One option might be to override to_xml for code accessors to add the check... hmm.
The SimpleProduct/APAProduct wrappers are useful, but necessarily
incomplete. It's useful to have a way to add an "interpretive" layer
above ONIX::Product, like APAProduct does, but in a more ad hoc way.
These interpretation modules can be chunked in whatever way makes sense
("all title methods" or "all getter methods", etc). I think APAProduct
can then be built up from a simple library of modules, but that can wait
another day.
This is a bit insane, but ah well, halfway done. Everything should be complete up to PR.13 of 2.1. And ONIX 3 is completely different, of course. Maybe we do need a code generation tool. Hm.
Also removed duplication by creating ProductBase and NameBase.
When declaring onix_code(s)_from_list, you can optionally include the :enforce option. By default, invalid data returns a code with key and value set to nil. If :enforce is true, invalid data raises an exception. If :enforce is false, the returned code has both key and value set to the invalid data.
And territory codes.
Conflicts: onix.gemspec
|
Thanks for this, there's plenty of food for thought. I'm flat out at the moment, but will review it when I can and consider what I think we should do |
Same functionality is now provided by ActiveSupport.
|
Tonight I needed support for setting and extracting discount codes on the Price composite. I thought it was a good excuse to review this pull request but I ended up getting a bit overwhelmed by the size of it and released a small update to 0.9.1 as an interim measure. Your code aesthetics are different to mine, but in general I like the new features you added. However I'm also daunted by the thought of supporting so much code that I'm not familiar with - I'm already coding too much on weekends and I doubt this would help. Are you interested in some sort of co-maintenance arrangement? I think that would make me happier to merge this as is. |
|
... so is one or the other of these two libraries preferred now? |
|
Not officially. I still use the official gems in my project, but I'm aware the API only provides access to a (very) limited subset of the ONIX spec. @joseph's changes support most (all?) of the spec, so you might have more luck trying his branch. I'm sorry for the state of the official gems, but I just don't have the time to actively maintain them very well. |
Okay, OBVIOUSLY this is an insane pull request. But it might start a discussion about whether it's better to pull in the work or to just maintain forks.
My changes all mesh together, which means splitting this into multiple pull requests is not likely to work very well.
First: I have tried to preserve full backwards compatibility. There are one or two very minor changes to specs, but basically, existing consumers of this library should continue to work just fine.
Anyway, an overview:
xml_accessorfor child objects, dates, country codes, boolean elements, etc (seeelements.rb)ONIX::Code-- keys and values from the lists are now accessibleSo, uh, how do we get this to work?