-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Update versioning/deprecations policy #17370
Conversation
spec/amp-versioning-policy.md
Outdated
- If a change is backward compatible, no version should be changed. | ||
- This means that, for now, there is no known case under which the minor version of an extension would be changed. | ||
- Minor versions may be changed for breaking behavior changes *without* breaking API changes. | ||
- Major versions *must* be changed for any breaking API changes. |
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.
@cramforce Alternative: Remove useless minor version from extensions moving forward.
spec/amp-versioning-policy.md
Outdated
- Changes to CSS included in AMP that are not backward compatible. | ||
- Changes to the DOM structure (elements, attributes and attribute values) generated by AMP extensions that are not backward compatible. | ||
|
||
The following special cases are not considered breaking changes: | ||
|
||
- Changes to elements and their children including their CSS if the element name starts with `i-`. | ||
- Changes to attribute and class names starting with `-`. But this doesn't mean, CSS backward compatibility may be broken with this mechanism, where it would otherwise be considered a breaking change. | ||
- Changes to attribute and class names starting with `i-` (but this doesn't mean that CSS backward compatibility may be broken with this mechanism, which would be considered a breaking change). | ||
- Changes required to maintain the security of AMP pages. |
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.
Security changes can be breaking by the definition above. I think this should come out of here and a note added that says something to the effect of "Changes required to maintain the security of AMP pages may not follow this policy if doing so would be detrimental to security"
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.
Moved the security point to its own subsection.
|
||
- Changes to the semantics of supported attributes and other exposed APIs. | ||
- Changes to exposed API semantics, e.g. supported attributes in an AMP extension. | ||
- Changes to CSS included in AMP that are not backward compatible. | ||
- Changes to the DOM structure (elements, attributes and attribute values) generated by AMP extensions that are not backward compatible. |
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.
Call out changes to what is considered a valid document?
spec/amp-versioning-policy.md
Outdated
|
||
## Breaking changes | ||
|
||
Breaking changes are: | ||
Breaking changes must be implemented in a new version. Breaking changes are defined as: |
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.
new version of amp or new version of extension?
How would this rule apply to a change to validation of non-extension html? For a concrete example, I'm trying to understand how these rules would apply to changing the syntax of the AMP boilerplate css.
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.
new version of amp or new version of extension?
Made this clearer.
How would this rule apply to a change to validation of non-extension html?
I think boilerplate CSS would fall under the new validation breaking change line item.
spec/amp-versioning-policy.md
Outdated
- Changes required to maintain the security of AMP pages. | ||
- Changes that break fewer than 0.1% of crawler accessible AMP pages. | ||
- Changes that break fewer than 0.1% of crawler-accessible AMP pages. |
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.
'break' / 'breaking' isn't well defined here. It's easy for questions of validation.
How would this apply to a behavioral change? For a contrived example, if all amp documents were suddenly modified so that the default text color was green. Or if all images were automatically given alt tags?
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.
Good point. This policy doesn't yet consider "enhancing" Cache transformations e.g. srcset
writing and blurry image placeholders. I'll add some language to accommodate that.
spec/amp-versioning-policy.md
Outdated
|
||
## Deprecation process | ||
## Deprecations |
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.
How do deprecations that are breaking interact with the rules for breaking changes above?
If the deprecation process is followed to introduce a breaking change, are the same requirements still at play (security -or- < 0.1%), or does the deprecation process represent an alternative route / exception to making breaking changes?
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.
The latter. Made this clearer, thanks.
|
||
The process is | ||
In rare cases, the AMP project may decide that an existing feature or API must be removed. Such breaking changes must follow the AMP deprecations process prior to removal. Deprecations must be publicly discussed and provide significant user benefit that justifies additional work for page developers. |
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.
Please clarify that features should never be deprecated before the replacement has been available for at least N weeks (N should be bigger than 2)
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.
The subsection below spells this out:
- A version must not be deprecated until a new version is released and stable for at least 1 month.
- A version must not be invalidated until it has been deprecated for at least 1 year.
* Update amp-versioning-policy.md * Minor edits * Add some links, tweak language. * Tweak goals. * s/may/must, do not allow removal of binaries. * Remove semver reference. * Clarify breaking changes, version removal. * Make security breaking changes clearer. * Validation can be breaking, relate to deprecations * Clean up version deprecations language. * Move supscript position. * Missing a word, clarification.
Context: #16691 (comment)
Changes in this PR
Other change proposals
Search Console warning
Current:
Suggested:
Validator warning
Current:
Suggested: