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

Extension Naming and Versioning #13

Merged
merged 10 commits into from
Aug 18, 2020
20 changes: 11 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
# OCFL Community Extensions

This repository is intended as a place for community extensions to the [OCFL Specification and Implementation Notes](https://ocfl.io/). Community extensions are intended as a way to share and collaborate outside of the specification process. They are intended to be citable and mostly static once published. Substantial revisions of content beyond simple fixes warrants publishing a new extension, perhaps marking the old one deprecated. Anybody may propose a community extension via a pull-request (PR) against this repository. The community will review and discuss it before it is merged in.
This repository is intended as a place for community extensions to the [OCFL Specification and Implementation Notes](https://ocfl.io/). Community extensions are intended as a way to share and collaborate outside of the specification process. They are intended to be citable and mostly static once published. Substantial revisions of content beyond simple fixes warrants publishing a new extension, marking the old one obsoleted by updating the *Obsoletes/Obsoleted by* sections in the new and old extension documents respectively. Anybody may propose a community extension via a pull-request (PR) against this repository. The community will review and discuss it before it is merged in.

The current set of extensions can be read on [GitHub Pages](https://ocfl.github.io/extensions/).

See also [pending pull requests](https://github.com/OCFL/extensions/pulls) for extensions under discussion.

## Organization of this repository

Community extensions should be written as GitHub flavored markdown in the `docs` directory of this repository. They should be numbered sequentially using a 4-digit, zero-padded prefix; should use hyphens to separate words; and have the `.md` extension.
Community extensions should be written as GitHub flavored markdown in the `docs` directory of this repository. They should be numbered sequentially using a 4-digit, zero-padded decimal prefix; should use hyphens to separate words; and have the `.md` extension. New, or substantially revised, extensions MUST use the next available number based on extensions current at the time of merging.

An example/template is available in this repository as [OCFL Community Extension](docs/0000-example-extension) and is rendered
via GitHub pages as https://ocfl.github.io/extensions/0000-example-extension
Expand All @@ -29,22 +29,24 @@ length limit is based on a survey of the defaults for various JSON parsers.
extension which MUST reference all the parameters.
* Type: Data type for the parameter. In order to allow validation and limit the scope for implementation specific variations,
parameters are typed.
* integer - may be signed or not as specified in the range parameter.
* integer - may be signed or not as specified in the range property.
* string - aligned with JSON strings, these should be UTF-8 encoded and avoid control characters.
* enumerated - one of an ordered set of labels which MUST conform to the same limitations as parameter names.
* boolean - may have the values false or true (lower case and unquoted as in JSON)
* 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, try to keep strings shorter than 4095 characters.
* boolean - MUST have the value *false* or *true* (lower case and unquoted as in JSON)
* Range: Further qualifies the valid values for a paramater 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.
Copy link
Member

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.

Copy link
Contributor

@rosy1280 rosy1280 Aug 18, 2020

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.

Copy link
Contributor

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

* For enumerated parameters, the range is a comma separated ordered list of labels that are valid for the parameter. Enumerated parameters are case sensitive and MUST always be quoted, so they are JSON strings.
* Default: Default value for parameter, which MUST be consistent with the range limitations. If this is left blank then the parameter is mandatory
* Default: Default value for parameter, which MUST be consistent with the range limitations. If this is left blank then the parameter is mandatory (if parameters are defined)

## 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:
Copy link
Contributor

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.

  1. Is it acceptable to put these JSON files anywhere other than in the extensions directory?
  2. If it is put inside the extensions directory, does it matter where? For example, extensions/0000-example-extension.json or extensions/0000-example-extension/0000-example-extension.json.

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. At the moment, there isn't another way of referencing extensions other than the extension directory so no.
  2. Interesting point, the intent was the extension directory and not a subdirectory.

Copy link
Member Author

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.

Copy link
Contributor

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.

Copy link
Member Author

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...

Copy link
Contributor

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


"0000-example-extension.md": {
Copy link
Contributor

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"  
}

Copy link
Member

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.

Copy link
Contributor

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.

Copy link
Member

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.

Copy link
Contributor

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.

Copy link
Contributor

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)

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor

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?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Done: #22

"firstExampleParameter": 12,
"secondExampleParameter": "Hello",
"thirdExampleParameter": "Green"
}
There MUST only be one parameter set defined for each extension.

An extension MAY be referenced without providing any paramaters, even if they are defined for that extension. However, if parameters are defined they MUST be complete, with valid values specified for all parameters that do not have a default. There MUST only be one parameter set defined for each extension reference.
5 changes: 4 additions & 1 deletion docs/0001-digest-algorithms.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,7 @@ This extension is an index of additional digest algorithms. It provides a contro

## Maintenance

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
Copy link
Contributor

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?

Copy link
Member Author

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.

Copy link
Member Author

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.

* If a long algorithm description is required it may be appropriate to submit an additional new extension describing the algorithm along with an update to this extension that links to it.