Skip to content
This repository has been archived by the owner on Nov 10, 2022. It is now read-only.

Issue #173 follow-up - Move Extensions text into a separate subsection #244

Closed
wants to merge 4 commits into from

Conversation

robstradling
Copy link
Contributor

No description provided.

} Extension;
~~~~~~~~~~~

The `Extension` structure provides a generic extensibility for log entries,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: The term 'log entries' here is a bit vague: SCTs and STHs are not log entries. They are artifacts produced by the log. Maybe it'd be better to replace it with 'log artifacts' ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't say that SCTs and STHs are log entries. "log entries" is the first of three items in a list; the second item is SCTs, and the third item is STHs.

However, to make it clear, I'll do "s/entries,/artifacts, including/".

IANA is asked to establish a registry of SCT extensions, named "CT Extension
Types for SCT", that initially consists of:
IANA is asked to establish a registry of `ExtensionType` values, named "CT
Extension Types", that initially consists of:
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider renaming the registry to CT log artifact extension types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK.

Copy link
Contributor

@eranmes eranmes left a comment

Choose a reason for hiding this comment

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

LGTM with two small suggestions.

IANA is asked to establish a registry of SCT extensions, named "CT Extension
Types for SCT", that initially consists of:
IANA is asked to establish a registry of `ExtensionType` values, named "CT Log
Artifact ExtensionTypes", that initially consists of:
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: "Extension Types" (two words)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"ExtensionType" is one word though, and it matches IANA is asked to establish a registry of `VersionedTransType` values, named "CT VersionedTransTypes.

i.e., We didn't write named "CT VersionedTrans Types"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, let's: 1) call it "CT Log Artifact Extensions", and 2) rename the "Value" column to "ExtensionType", and 3) rename the "Extension" column to "Status".

Copy link
Contributor

@eranmes eranmes left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@eranmes eranmes left a comment

Choose a reason for hiding this comment

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

Latest revision LGTM.

@robstradling
Copy link
Contributor Author

Merged at d4fb5ce

@robstradling robstradling deleted the extensions_section branch April 6, 2017 13:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants