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

Add fixture for invalid head version format #80

Merged
merged 2 commits into from
May 2, 2021

Conversation

pwinckles
Copy link
Contributor

Fixture for invalid head version number

Copy link
Collaborator

@zimeon zimeon left a comment

Choose a reason for hiding this comment

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

I'm flagging this a "request changes" because I think we need to talk about expectations here. My sense is that any validation should short circuit here and not look into versions with invalid version numbers 1. Might also have implications on some implementation note about validation practices.

@pwinckles
Copy link
Contributor Author

@zimeon What's the path forward on this one?

@zimeon
Copy link
Collaborator

zimeon commented Apr 26, 2021

What do you think the key error condition for this fixture is? I find it hard to decide but I'm pretty sure that head matching the one "version-number" is really a side issue to the fact the there are no valid versions in the root inventory. There are also no valid version directories if one started from there. I don't see E011 as a likely error code, could imagine E008 or E009?

@pwinckles
Copy link
Contributor Author

@zimeon I don't think there is a good code. See OCFL/spec#532

@zimeon
Copy link
Collaborator

zimeon commented Apr 27, 2021

Since in v1.0 we do not have a good error code, I propose that we just got with the catchall E001. What do you think @pwinckles ?

@pwinckles
Copy link
Contributor Author

Works for me.

I renamed the fixture to E001_invalid_version_format

@zimeon zimeon requested a review from awoods April 27, 2021 16:28
@awoods awoods merged commit b1fb7e4 into OCFL:main May 2, 2021
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