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

Fixed: Specify that EFPs must be relative to OR. #228

Merged
merged 5 commits into from
Oct 16, 2018

Conversation

ahankinson
Copy link
Contributor

This PR specifies that existing file paths MUST be given relative to the Object Root.

Copy link
Member

@awoods awoods left a comment

Choose a reason for hiding this comment

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

Although not harmful, it seems that is added sentence is redundant with the definition of "existing file path", introduced in: https://github.com/OCFL/spec/pull/225/files

@ahankinson
Copy link
Contributor Author

It is mentioned in the terminology section, but I think we decided that normative language is not used in terminology. So this is primarily to apply the normative language to the requirement for EFPs.

@zimeon
Copy link
Contributor

zimeon commented Oct 12, 2018

I agree with @ahankinson that it is good to make this clear statement in the main text, even if it duplicates the definition. I guess this will have conflicts given other LFP->EFP changes. I suggest that the sentence should be part of the previous paragraph rather than living on its own.

@awoods
Copy link
Member

awoods commented Oct 12, 2018

The extra clarity works for me.

@@ -515,6 +515,10 @@ <h2>Manifest</h2>
containing the <a>logical file path</a>s of files in the OCFL Object that have content with the
given digest.
</p>
<p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest combine into preceding para but otherwise good by me

ahankinson added a commit that referenced this pull request Oct 16, 2018
@ahankinson
Copy link
Contributor Author

The changes here will conflict with changes from fixing #221, in PR #225. I've rolled the changes into that PR, so I will close this one.

@ahankinson ahankinson closed this Oct 16, 2018
@ahankinson
Copy link
Contributor Author

sigh ignore me. Re-opening b/c that one was already merged.

@ahankinson ahankinson reopened this Oct 16, 2018
@ahankinson
Copy link
Contributor Author

@awoods @zimeon could you merge this please?

@zimeon
Copy link
Contributor

zimeon commented Oct 16, 2018

Editorial change, merging on 3...

@zimeon zimeon merged commit 359aeb6 into master Oct 16, 2018
@zimeon zimeon deleted the fixed-specify-relative-path branch October 16, 2018 15:51
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.

3 participants