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

feat(181): Import AEP-181 from AIP-181 #150

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

toumorokoshi
Copy link
Member

notable differences are:

  • removed all references to Google.
  • slight change of formatting in "emergency changes", to clearly outline scenarios where such change are acceptable.

notable differences are:

- removed all references to Google.
- slight change of formatting in "emergency changes", to clearly
  outline scenarios where such change are acceptable.
@toumorokoshi toumorokoshi requested a review from a team as a code owner March 18, 2024 05:18
Copy link
Collaborator

@rofrankel rofrankel left a comment

Choose a reason for hiding this comment

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

My one comment is about something that exists in the Google AIP as well. I find it kind of confusing, but it doesn't need to block the PR, so approving.

Comment on lines 59 to 61
users. (Creating a new major version is an inconvenience to all users.) In this
case, the API producer **may** deprecate the component, but **must** continue
to support the component for the normal turndown period for a stable component.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't really understand how deprecation helps here. If I make a breaking change (e.g. delete a field from a resource), what would I deprecate, and what would I "continue to support"?

Copy link

@cfineman cfineman left a comment

Choose a reason for hiding this comment

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

I had a few questions. I'm not "officially" part of the project at this point but these were of interest to me.

aep/general/0181/aep.md.j2 Show resolved Hide resolved
breaking change, if this will only cause inconvenience to a small subset of
users. (Creating a new major version is an inconvenience to all users.) In this
case, the API producer **may** deprecate the component, but **must** continue
to support the component for the normal turndown period for a stable component.

Choose a reason for hiding this comment

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

Is the "turndown" period specified elsewhere? I have not seen it yet... presumably this would be formally stated somewhere.

Co-authored-by: Alex Stephen <1325798+rambleraptor@users.noreply.github.com>
Copy link
Contributor

@mkistler mkistler left a comment

Choose a reason for hiding this comment

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

Looks good. 👍

an extreme course of action, and should be treated with equal or greater gravity
as creating a new major version.

### Emergency changes
Copy link
Member Author

Choose a reason for hiding this comment

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

We'l remove this from the AEP - since it's an implicit understanding of all software.

of them individually.

Breaking changes **must** be both allowed and expected in alpha components, and
users **must** have no expectation of stability.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
users **must** have no expectation of stability.
users **must not** have an expectation of stability.

WDYT?

Because users of beta components tend to have a lower tolerance of change, beta
components **should** be as stable as possible; however, the beta component
**must** be permitted to change over time. These changes **should** be minimal
but **may** include backwards-incompatible changes to beta components.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
but **may** include backwards-incompatible changes to beta components.
but **may** include backwards-incompatible changes.

(seems redundant)

but **may** include backwards-incompatible changes to beta components.
Backwards-incompatible changes **must** be made only after a reasonable
deprecation period to provide users with an opportunity to migrate their code.
This deprecation period **must** be defined at the time of being marked beta.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
This deprecation period **must** be defined at the time of being marked beta.
This deprecation period **must** be defined at the time the component is declared beta.

This is more consistent with the above phrasing: "A beta component must be considered complete and ready to be declared stable..."


Beta components **should** be time-boxed and promoted to stable if no issues
are found in the specified timeframe, which **should** be specified at the time
of being marked beta. A reasonable time period **may** vary, but a good rule of
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
of being marked beta. A reasonable time period **may** vary, but a good rule of
of being declared beta. A reasonable time period **may** vary, but a good rule of

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants