Capitalize RFC 2119 keywords when appropriate #94

kaelig · 2022-01-10T05:15:22Z

Fixes #77

This PR also introduces a few edits, to try and use less ambiguous keywords when possible, or to make it clear that one of the sections isn't normative.

netlify · 2022-01-10T05:15:30Z

✔️ Deploy Preview for dtcg-tr ready!

🔨 Explore the source changes: 29ba4a3

🔍 Inspect the deploy log: https://app.netlify.com/sites/dtcg-tr/deploys/61eefc6aab616600073c8f4e

😎 Browse the preview: https://deploy-preview-94--dtcg-tr.netlify.app

c1rrus

Thanks for doing this. I've queried some of the changes, but not 100% sure about them TBH. It's quite subjective as to what is or isn't something that affects interoperability.

c1rrus · 2022-01-11T23:01:16Z

technical-reports/format/aliases.md

@@ -7,7 +7,7 @@ Aliases are useful for:
 - Expressing design choices
 - Eliminating repetition of values in token files (DRYing up the code)

-For a design token to reference another, its value should be a string containing the period-separated (.) path to the token it's referencing enclosed in curly brackets.


Perhaps this needs to be MUST rather than SHOULD. Parsers should reject reference that don't conform to the syntax we're defining here.

I agree. Change to "MUST".

c1rrus · 2022-01-11T23:03:36Z

technical-reports/format/aliases.md


-Aliases may reference other aliases. In this case, tools should follow each reference until they find a token with an explicit value. Circular references are not allowed. If a design token file contains circular references, then the value of all tokens in that chain is unknown and an appropriate error or warning message should be displayed to the user.
+Aliases MAY reference other aliases. In this case, tools SHOULD follow each reference until they find a token with an explicit value. Circular references are not allowed. If a design token file contains circular references, then the value of all tokens in that chain is unknown and an appropriate error or warning message SHOULD be displayed to the user.


Maybe this needs to be stricter too: "In this case, tools MUST follow each reference..."

Agree, first SHOULD on line 35 should change to MUST.

c1rrus · 2022-01-11T23:14:42Z

technical-reports/format/design-token.md

@@ -62,7 +62,7 @@ Due to the syntax used for [token aliases](#aliases-references) the following ch

 ### Token value type

-Token values may be any valid JSON type:
+Token values MUST be a valid JSON type:


Hmm... re-reading this now, I'm think we don't need this "Token value type" section at all.

The "Type" chapter later on explains things more clearly. As it says there, every token has an unambiguous type (which inlude the basic JSON types like string, number, etc.) and there is an algorithm for determining it.

This section doesn't add anything useful and, if anything, might confuse people.

I agree, and the statement "Token values MUST be a valid JSON type" is inaccurate since we're defining a number of additional types beyond the standard JSON set.

c1rrus · 2022-01-11T23:18:29Z

technical-reports/format/design-token.md


 <div class="issue" data-number="55" title="Object vs Array">

 The structure in the example above is a JSON object, an **unordered** set of name/value pairs.

 - Objects can't contain members with duplicate keys
- Ordering of object members may not be preserved (as per [RFC 7159](https://tools.ietf.org/html/rfc7159#section-4)), meaning token retrieval may or may not result in the same ordering as the input
+- Ordering of object members MAY NOT be preserved (as per [RFC 7159](https://tools.ietf.org/html/rfc7159#section-4)), meaning token retrieval MAY or MAY NOT result in the same ordering as the input


I don't think these need to be capitalised "MAY"s. This line is just highlighting something we inherit from the underlying JSON spec. We're not actually specifying something that implementors MAY do here.

Perhaps a rewrite:

Ordering of object members is not guaranteed (as per RFC 7159)

c1rrus · 2022-01-11T23:23:32Z

technical-reports/format/design-token.md


 ## Description

 A plain text description explaining the token's purpose. Tools MAY use the description in various ways.

 For example:

- Style guide generators could display the description text alongside a visual preview of the token


These are just examples to illustrate how description may be used or persented in different kinds of tools.

RFC 2119 states:

Imperatives of the type defined in this memo must be used with care
and sparingly. In particular, they MUST only be used where it is
actually required for interoperation or to limit behavior which has
potential for causing harm (e.g., limiting retransmisssions) For
example, they must not be used to try to impose a particular method
on implementors where the method is not required for
interoperability.

I don't think these examples affect interoperability, so I think they shouldn't use capitalised imperatives.

I agree. This feels like an overreach of a find/replace.

Actually, looking at this more closely, I think the MAY's make sense here. Yes, they are examples, but they're describing potential tool functionality and using MAY helps with interoperability since it's not required functionality. I may be reading this wrong though.

Fair point. I'm inclined to agree with your @kevinmpowell

FWIW, reviewing this PR has made me realise applying RFC 2119 appropriately is a lot harder than it looks!

c1rrus · 2022-01-12T20:44:46Z

technical-reports/format/terminology.md

@@ -11,7 +11,7 @@ Few examples:
 - `color-text-primary: #000000;`
 - `font-size-heading-level-1: 44px;`

-The name may be associated with additional [Token Properties](#design-token-properties).


I don't think we need uppercase MAY here as this is the terminology section, which is just descriptive. We're not specifying anything that implementors or authors need to do here.

c1rrus · 2022-01-12T20:48:32Z

technical-reports/format/types.md

@@ -58,7 +58,7 @@ $translucent-shadow: hsla(300, 100%, 50%, 0.5);

 ## Dimension

-Represents an amount of distance in a single dimension in the UI, such as a position, width, height, radius, or thickness. The `type` property must be set to the string "dimension". The value must be a string containing a number (either integer or floating-point) followed by either a "px" or "rem" unit (future spec iterations may add support for additional units).
+Represents an amount of distance in a single dimension in the UI, such as a position, width, height, radius, or thickness. The `type` property must be set to the string "dimension". The value must be a string containing a number (either integer or floating-point) followed by either a "px" or "rem" unit (future spec iterations MAY add support for additional units).


I don't think this one needs to be uppercase, as this is just an FYI. We're not specifying anything for implementors or authors in order for them to conform to this spec version.

c1rrus · 2022-01-12T21:33:35Z

technical-reports/format/types.md


 ## Font name

 <div class="issue" data-number="53">

-A naive approach like the one below may be appropriate for the first stage of the specification, but this may be more complicated than it seems due to platform/OS/browser restrictions.
+A naive approach like the one below MAY be appropriate for the first stage of the specification, but this could be more complicated than it seems due to platform/OS/browser restrictions.


Probably don't need uppercase MAY here either, since is just an issue description.

kevinmpowell

I agree with @c1rrus' feedback here. This is largely solid, but there's a few cases where "may" is used descriptively.

kevinmpowell · 2022-01-24T18:48:43Z

technical-reports/format/aliases.md

@@ -7,7 +7,7 @@ Aliases are useful for:
 - Expressing design choices
 - Eliminating repetition of values in token files (DRYing up the code)

-For a design token to reference another, its value should be a string containing the period-separated (.) path to the token it's referencing enclosed in curly brackets.


I agree. Change to "MUST".

kevinmpowell · 2022-01-24T18:49:17Z

technical-reports/format/aliases.md


-Aliases may reference other aliases. In this case, tools should follow each reference until they find a token with an explicit value. Circular references are not allowed. If a design token file contains circular references, then the value of all tokens in that chain is unknown and an appropriate error or warning message should be displayed to the user.
+Aliases MAY reference other aliases. In this case, tools SHOULD follow each reference until they find a token with an explicit value. Circular references are not allowed. If a design token file contains circular references, then the value of all tokens in that chain is unknown and an appropriate error or warning message SHOULD be displayed to the user.


Agree, first SHOULD on line 35 should change to MUST.

kevinmpowell · 2022-01-24T18:50:17Z

technical-reports/format/design-token.md

@@ -62,7 +62,7 @@ Due to the syntax used for [token aliases](#aliases-references) the following ch

 ### Token value type

-Token values may be any valid JSON type:
+Token values MUST be a valid JSON type:


I agree, and the statement "Token values MUST be a valid JSON type" is inaccurate since we're defining a number of additional types beyond the standard JSON set.

kevinmpowell · 2022-01-24T18:51:37Z

technical-reports/format/design-token.md


 <div class="issue" data-number="55" title="Object vs Array">

 The structure in the example above is a JSON object, an **unordered** set of name/value pairs.

 - Objects can't contain members with duplicate keys
- Ordering of object members may not be preserved (as per [RFC 7159](https://tools.ietf.org/html/rfc7159#section-4)), meaning token retrieval may or may not result in the same ordering as the input
+- Ordering of object members MAY NOT be preserved (as per [RFC 7159](https://tools.ietf.org/html/rfc7159#section-4)), meaning token retrieval MAY or MAY NOT result in the same ordering as the input


Perhaps a rewrite:

Ordering of object members is not guaranteed (as per RFC 7159)

kevinmpowell · 2022-01-24T18:52:24Z

technical-reports/format/design-token.md


 ## Description

 A plain text description explaining the token's purpose. Tools MAY use the description in various ways.

 For example:

- Style guide generators could display the description text alongside a visual preview of the token


I agree. This feels like an overreach of a find/replace.

kevinmpowell · 2022-01-24T18:52:37Z

technical-reports/format/terminology.md

@@ -11,7 +11,7 @@ Few examples:
 - `color-text-primary: #000000;`
 - `font-size-heading-level-1: 44px;`

-The name may be associated with additional [Token Properties](#design-token-properties).


kevinmpowell · 2022-01-24T18:52:52Z

technical-reports/format/types.md

@@ -58,7 +58,7 @@ $translucent-shadow: hsla(300, 100%, 50%, 0.5);

 ## Dimension

-Represents an amount of distance in a single dimension in the UI, such as a position, width, height, radius, or thickness. The `type` property must be set to the string "dimension". The value must be a string containing a number (either integer or floating-point) followed by either a "px" or "rem" unit (future spec iterations may add support for additional units).
+Represents an amount of distance in a single dimension in the UI, such as a position, width, height, radius, or thickness. The `type` property must be set to the string "dimension". The value must be a string containing a number (either integer or floating-point) followed by either a "px" or "rem" unit (future spec iterations MAY add support for additional units).


kevinmpowell · 2022-01-24T18:53:03Z

technical-reports/format/types.md


 ## Font name

 <div class="issue" data-number="53">

-A naive approach like the one below may be appropriate for the first stage of the specification, but this may be more complicated than it seems due to platform/OS/browser restrictions.
+A naive approach like the one below MAY be appropriate for the first stage of the specification, but this could be more complicated than it seems due to platform/OS/browser restrictions.


kevinmpowell · 2022-01-24T19:23:12Z

@c1rrus and @kaelig I've reconciled almost all the feedback provided and just submitted a commit with updates. I think merging this prior to merging the other open PRs will be the best way to avoid a lot of conflicts.

c1rrus · 2022-01-25T17:20:55Z

@c1rrus and @kaelig I've reconciled almost all the feedback provided and just submitted a commit with updates. I think merging this prior to merging the other open PRs will be the best way to avoid a lot of conflicts.

Yup. Seconded. The sooner we can get this merged, the better

@kaelig

SHA: 52a7dce Reason: push, by @kaelig Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

@kaelig

SHA: 52a7dce Reason: push, by @kaelig Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

kaelig force-pushed the fix-77 branch 2 times, most recently from 21fff16 to 6048988 Compare January 10, 2022 05:38

Capitalize RFC 2119 keywords when appropriate

0eee368

kaelig force-pushed the fix-77 branch from 6048988 to 0eee368 Compare January 10, 2022 06:25

kaelig mentioned this pull request Jan 10, 2022

Capitalize RFC 2119 keywords when appropriate [redux] #96

Closed

kaelig requested review from c1rrus, ChucKN0risK, kevinmpowell, dbanksdesign and donnavitan January 10, 2022 23:46

kaelig added this to the Next Review Cycle milestone Jan 10, 2022

c1rrus requested changes Jan 12, 2022

View reviewed changes

kevinmpowell requested changes Jan 24, 2022

View reviewed changes

Update casing of RFC 2119 keywords

29ba4a3

c1rrus approved these changes Jan 25, 2022

View reviewed changes

kevinmpowell approved these changes Jan 25, 2022

View reviewed changes

kaelig merged commit 52a7dce into main Jan 25, 2022

kaelig deleted the fix-77 branch January 25, 2022 17:40

github-actions bot added a commit that referenced this pull request Jan 25, 2022

Capitalize RFC 2119 keywords when appropriate (#94)

ceec499

SHA: 52a7dce Reason: push, by @kaelig Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

github-actions bot added a commit that referenced this pull request Jan 25, 2022

Capitalize RFC 2119 keywords when appropriate (#94)

086dcfa

SHA: 52a7dce Reason: push, by @kaelig Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Capitalize RFC 2119 keywords when appropriate #94

Capitalize RFC 2119 keywords when appropriate #94

kaelig commented Jan 10, 2022 •

edited

Loading

netlify bot commented Jan 10, 2022 •

edited

Loading

c1rrus left a comment

c1rrus Jan 11, 2022

kevinmpowell Jan 24, 2022

c1rrus Jan 11, 2022

kevinmpowell Jan 24, 2022

c1rrus Jan 11, 2022

kevinmpowell Jan 24, 2022

c1rrus Jan 11, 2022

kevinmpowell Jan 24, 2022

c1rrus Jan 11, 2022

kevinmpowell Jan 24, 2022

kevinmpowell Jan 24, 2022

c1rrus Jan 25, 2022

c1rrus Jan 12, 2022

kevinmpowell Jan 24, 2022

c1rrus Jan 12, 2022

kevinmpowell Jan 24, 2022

c1rrus Jan 12, 2022

kevinmpowell Jan 24, 2022

kevinmpowell left a comment

kevinmpowell Jan 24, 2022

kevinmpowell Jan 24, 2022

kevinmpowell Jan 24, 2022

kevinmpowell Jan 24, 2022

kevinmpowell Jan 24, 2022

kevinmpowell Jan 24, 2022

kevinmpowell Jan 24, 2022

kevinmpowell Jan 24, 2022

kevinmpowell commented Jan 24, 2022

c1rrus commented Jan 25, 2022


		Aliases may reference other aliases. In this case, tools should follow each reference until they find a token with an explicit value. Circular references are not allowed. If a design token file contains circular references, then the value of all tokens in that chain is unknown and an appropriate error or warning message should be displayed to the user.
		Aliases MAY reference other aliases. In this case, tools SHOULD follow each reference until they find a token with an explicit value. Circular references are not allowed. If a design token file contains circular references, then the value of all tokens in that chain is unknown and an appropriate error or warning message SHOULD be displayed to the user.

Capitalize RFC 2119 keywords when appropriate #94

Capitalize RFC 2119 keywords when appropriate #94

Conversation

kaelig commented Jan 10, 2022 • edited Loading

netlify bot commented Jan 10, 2022 • edited Loading

c1rrus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinmpowell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinmpowell commented Jan 24, 2022

c1rrus commented Jan 25, 2022

kaelig commented Jan 10, 2022 •

edited

Loading

netlify bot commented Jan 10, 2022 •

edited

Loading