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

Use plain OM::XML::Document without the datastream #306

Merged
merged 1 commit into from Aug 23, 2016
Merged

Use plain OM::XML::Document without the datastream #306

merged 1 commit into from Aug 23, 2016

Conversation

awead
Copy link
Contributor

@awead awead commented Aug 22, 2016

This supports ActiveFedora 11 and removes the dependency on OmDatastream.

@awead
Copy link
Contributor Author

awead commented Aug 22, 2016

ping @mjgiarlo @escowles

@escowles
Copy link
Contributor

This looks good to me — the build errors can probably be fixed with: https://github.com/projecthydra/hydra-works/blob/06d5cba043b1bfdb1e7570ec189cc446cc01e3fe/.travis.yml#L9-L10

@coveralls
Copy link

coveralls commented Aug 22, 2016

Coverage Status

Coverage increased (+0.003%) to 99.012% when pulling 0e4a7a1 on af-11 into 969eaca on master.

@@ -21,6 +21,8 @@ Gem::Specification.new do |spec|
spec.add_dependency 'hydra-pcdm', '>= 0.8'
spec.add_dependency 'hydra-derivatives', '~> 3.0'
spec.add_dependency 'hydra-file_characterization', '~> 0.3', '>= 0.3.3'
spec.add_dependency 'active-fedora', '~> 11.0.0.rc7'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this PR require AF 11, or would it work without this constraint? I'm concerned about forcing AF 11 on folks before we have tooling in place for them to deal with unordered values.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this? This will force everyone to use rdf 2. Is there any reason it wouldn't work with version 10?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I required AF 11 so I could reproduce the original missing constant AF::OmDatastream error, but a datastream-less implementation should work fine under AF 10.

@awead
Copy link
Contributor Author

awead commented Aug 23, 2016

@jcoyne I removed the AF constraint, although I haven't tested this with Sufia master, it should fix the OmDatastream error.

@coveralls
Copy link

coveralls commented Aug 23, 2016

Coverage Status

Coverage increased (+0.003%) to 99.012% when pulling 197f417 on af-11 into 969eaca on master.

@jcoyne jcoyne merged commit 9790d22 into master Aug 23, 2016
@mjgiarlo mjgiarlo deleted the af-11 branch August 23, 2016 03:45
@mjgiarlo
Copy link
Member

👏

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

5 participants