-
Notifications
You must be signed in to change notification settings - Fork 12
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
Extension Naming and Versioning #13
Conversation
Clarify versioning and some tidying up
Clarified wording around maintenance
README.md
Outdated
|
||
## Referencing Parameters | ||
|
||
Wherever a parameterised extension is referenced, include any parameters in an accompanying JSON file. If using an extensions directory, the JSON file MUST be named for the extension and included in the directory. For example, the example extension above would have an accompanying file *0000-example-extension.json* which might contain: | ||
Wherever a parameterised extension is referenced, any parameters MAY be included in an accompanying JSON file. If using an extensions directory, the JSON file MUST be named for the extension and included in the directory. For example, the example extension above would have an accompanying file *0000-example-extension.json* which might contain: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little confused by this wording:
If using an extensions directory, the JSON file MUST be named for the extension and included in the directory.
- Is it acceptable to put these JSON files anywhere other than in the
extensions
directory? - If it is put inside the
extensions
directory, does it matter where? For example,extensions/0000-example-extension.json
orextensions/0000-example-extension/0000-example-extension.json
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- At the moment, there isn't another way of referencing extensions other than the extension directory so no.
- Interesting point, the intent was the extension directory and not a subdirectory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarified that it needs to be in the extension directory and not a subdirectory. Also, ocfl_layout.json is another place where a parameter sidecar file can occur.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@neilsjefferies Thanks for the updates!
The spec currently states that extensions should use a subdirectory of extensions
"named according to a registered extension name". I don't necessary have a problem with the json
files being directly under extensions
. In fact, for extensions that do not have additional files, this makes a lot of sense. But, it is a little more awkward for extensions that have both configuration and a sub directory. For example:
extensions/
- 0000-example-extension.json
- 0000-example-extension/
- ... some files ...
In those cases, it would be neater if the json file was under the extension directory. Perhaps, the json file should be at the root of extensions
when the extension has no other files and under a subdirectory if it does? Not a huge deal either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I agree! I intended that the extension parameters should be in the "root directory of the extension" not extensions since it shouldn't contain files as per the spec. Let me reword that...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me too
In order to have an additional digest algorithm listed here, please submit a pull request on this extension that adds it to the table. New entries should have a name that does not conflict with those defined in the [OCFL Specification](https://ocfl.io/latest/spec/) or this community extension, and is preferably in common use for the given algorithm. In the case that long description is required it may be appropriate to submit a new extension describing the algorithm along with an update to this extension that links to it. | ||
In order to have an additional digest algorithm listed here, please submit a pull request on this extension that: | ||
* Adds the algorithm to the table. New entries should have a name that does not conflict with those defined in the [OCFL Specification](https://ocfl.io/latest/spec/) or this community extension, and is preferably in common use for the given algorithm. | ||
* Creates a new version of this extension with the next available extension number, obsoleting the current one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume that this extension is no longer going to be parameterized, correct? So, there's effectively no way to reference it. Does it matter if a new version of this extension was created opposed to just updating it? If a new version was created, how would anyone know which version an object was using?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it will be parameterized but the PR hasn't been merged yet. Editors discussed versioning and decided to hold with RFC behaviour and create new extension numbers for additions. A new version doesn't cause the old one to vanish. Whatever version an object references will continue to be valid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update post 20200602 Editorial discussion. We're not parameterizing but the versioning description remains to be consistent with how extensions are handled in in general.
Updated to define registered name. |
README.md
Outdated
Wherever a parameterised extension is referenced, include any parameters in an accompanying JSON file. If using an extensions directory, the JSON file MUST be named for the extension and included in the directory. For example, the example extension above would have an accompanying file *0000-example-extension.json* which might contain: | ||
Wherever a parameterised extension is referenced, any parameters MAY be included in an accompanying sidecar JSON file that | ||
uses the registered name with the filename extension `.json`. If using an 'extensions' directory, the JSON file MUST be | ||
included at the top level of the directory and not a subdirectory. Another place that an extension may be referenced is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another place that an extension may be referenced is
ocfl_layout.json
where the sidecar file should be alongside it in the Storage Root.
Is this the equivalent of saying?
If
ocfl_layout.json
references an extension, then the extension's sidecar file must be in the Storage Root instead of the extensions directory.
Or should both locations be checked?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes (to your first question).
included at the top level of the directory and not a subdirectory. Another place that an extension may be referenced is | ||
`ocfl_layout.json` where the sidecar file should be alongside it in the Storage Root. | ||
|
||
For example, the example extension above would have an accompanying file *0000-example-extension.json* which might contain: | ||
|
||
"0000-example-extension.md": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps not appropriate for this PR, but I'm not sure the parameters json file should have this key "0000-example-extension.md". Wouldn't the contents of 0000-example-extension.md
(in this example) be:
{
"firstExampleParameter": 12,
"secondExampleParameter": "Hello",
"thirdExampleParameter": "Green"
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see potential value in keeping the extension name as the key in this JSON document... particularly in the case where an extension is chaining multiple other extensions together.
..or, for the chaining use case, the chained extensions may also be found in the parameter listing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless there's an intention to suggest that you could define multiple extensions in the same file, I don't think there's much value in keeping the key, which makes parsing a little kludgy. I'm not sure it would even work for chaining because you can't infer an ordinal relationship.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the reasons for keeping the key are hypothetical, at best... maybe it should be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is the only change that is being requested in this PR, perhaps we could allow this PR to move forward and make the change in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rosy1280, @neilsjefferies had expressed the intent to clarify the language around where the json
file is supposed to be written: #13 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But, yes, the key doesn't necessarily need to be addressed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parking this for a later pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pwinckles can you create a ticket for this issue? (or did you already?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: #22
README.md
Outdated
have the `.md` extension; and have a maximum of 250 characters. New, or substantially revised, extensions MUST use the next | ||
available number based on extensions current at the time of merging. | ||
|
||
The *Registered Name* of an extension is the name of the extension file in the `docs` directory, excluding the `.md` extension. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest the "Registered Name" to be the name provided in the header (https://github.com/OCFL/extensions/pull/16/files#diff-77315547024f9923f13fb0a37b4ecf3aR3), as opposed to the filename in the docs
directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, but do we then need some language saying that the extension file is also named according to the registered extension name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zimeon : Are you suggesting wording that aligns the filename with the registered extension name? If so, that sounds reasonable.
* boolean - MUST have the value *false* or *true* (lower case and unquoted as in JSON) | ||
* Range: Further qualifies the valid values for a parameter depending on its type. | ||
* For integer parameters, the range specifies minimum and maximum values, separated by a comma, which MUST be integers themselves. | ||
* For string parameters, the range specifies the maximum length of the string as an integer number of characters, not bytes. Again, based on a survey of parsers, strings SHOULD be shorter than 4095 characters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect the range
for string parameters to somehow define the allowable values. Would a regex or enumerated list make sense? I am not sure I see the benefit of using the range
property to define string lengths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parking this for a later pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@awoods to create an issue for discussion
included at the top level of the directory and not a subdirectory. Another place that an extension may be referenced is | ||
`ocfl_layout.json` where the sidecar file should be alongside it in the Storage Root. | ||
|
||
For example, the example extension above would have an accompanying file *0000-example-extension.json* which might contain: | ||
|
||
"0000-example-extension.md": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see potential value in keeping the extension name as the key in this JSON document... particularly in the case where an extension is chaining multiple other extensions together.
..or, for the chaining use case, the chained extensions may also be found in the parameter listing.
Updated Registered name definition and specified parameter location more clearly.
Added name
Added name
Clarified versioning/maintenance text in response to issue #4 (plus a couple of formatting/language fixes)