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

Replace OM with nokogiri-happymapper #346

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
4 participants
@jcoyne
Copy link
Member

jcoyne commented Jul 16, 2018

No description provided.

@jcoyne jcoyne added the in progress label Jul 16, 2018

@no-reply

This comment has been minimized.

Copy link
Member

no-reply commented Jul 16, 2018

Yes, yes, yes!

I'd love to get this in prior to #343.

@no-reply no-reply self-requested a review Jul 16, 2018

@cjcolvar

This comment has been minimized.

Copy link
Member

cjcolvar commented Jul 17, 2018

I'd like there to be more investigation and discussion before this gets merged so we can be very deliberate about our decision. I feel like there is still a fair bit of XML out in the community which is being parsed and generated via OM (within Avalon's ecosystem there definitely is). We still get occasional questions from people trying to work through the XML model lesson (https://github.com/samvera/hydra/wiki/Lesson---Build-a-Codex-model-with-XML). It would be great if we could adopt a single approach for reading and writing across samvera repos then update our documentation accordingly so we can provide some guidance.

This PR seems like one approach. I'd like to see how nokogiri-happymapper works for generating xml especially the tricky scenarios in OM like when you need to generate multiple nodes for a value via templates.

For what its worth nom-xml is another option.

@mark-dce

This comment has been minimized.

Copy link
Contributor

mark-dce commented Sep 20, 2018

@jcoyne are you committed to the happymapper solution? OR would you be OK with closing this in favor of #354 as a solution to removing the OM dependency without introducing an alternative dependency?

@jcoyne

This comment has been minimized.

Copy link
Member Author

jcoyne commented Sep 21, 2018

I don't care. The main goal was to get rid of OM. if #354 does that 👍

@mark-dce

This comment has been minimized.

Copy link
Contributor

mark-dce commented Sep 21, 2018

@jcoyne thanks! I'm gonna close this for now then.

@mark-dce mark-dce closed this Sep 21, 2018

@mark-dce mark-dce removed the in progress label Sep 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.