-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
{Core} Add support and document for Breaking Change Pre-announce #28832
base: dev
Are you sure you want to change the base?
Conversation
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
breaking change pre-announce |
def _next_breaking_change_version(): | ||
global __bc_version | ||
if __bc_version: | ||
return __bc_version | ||
cur_version = packaging.version.parse(__version__) | ||
next_bc_version = packaging.version.parse(NEXT_BREAKING_CHANGE_RELEASE) | ||
if cur_version >= next_bc_version: | ||
fetched_bc_version = _next_breaking_change_version_from_milestone(cur_version) | ||
if fetched_bc_version: | ||
__bc_version = fetched_bc_version | ||
return fetched_bc_version | ||
__bc_version = NEXT_BREAKING_CHANGE_RELEASE | ||
return NEXT_BREAKING_CHANGE_RELEASE |
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.
When we release a CLI version with NEXT_BREAKING_CHANGE_RELEASE forget to change, when the cli users will see a different version every half year. My recommendation is that change the NEXT_BREAKING_CHANGE_RELEASE version number in the CLI release pipeline. Every time we bump up the cli version, the pipeline will check and change the NEXT_BREAKING_CHANGE_RELEASE version.
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.
I agree with that. If we could update the NEXT_BREAKING_CHANGE_RELEASE
after breaking change release in pipeline, we don't need a fallback logic to get the version number.
### Pre-announce Breaking Changes | ||
|
||
All breaking changes **must** be pre-announced several sprints ahead Release. There are two approaches to inform both interactive users and automatic users about the breaking changes. | ||
|
||
1. (**Mandatory**) Breaking Changes must be pre-announced through Warning Log while executing. | ||
2. (*Automatic*) Breaking Changes would be collected automatically and listed in [Upcoming Breaking Change](https://learn.microsoft.com/en-us/cli/azure/upcoming-breaking-changes). | ||
|
||
## Workflow | ||
|
||
### Overview | ||
|
||
* CLI Owned Module | ||
* Service Team should create an Issue that requests CLI Team to create the pre-announcement several sprints ahead Breaking Change Window. The CLI team will look at the issue and evaluate if it will be accepted in the next breaking change release. | ||
* Please ensure sufficient time for CLI Team to finish the pre-announcement. | ||
* The pre-announcement should be released ahead of Breaking Change Window. | ||
* Service Owned Module | ||
* Service Team should create a Pull Request that create the pre-announcement several sprints ahead Breaking Change Window. | ||
* The pre-announcement should be released ahead of Breaking Change Window. | ||
* After releasing the pre-announcement, a pipeline would be triggered, and the Upcoming Breaking Change Documentation would be updated. | ||
* At the start of Breaking Change window, emails would be sent to notify Service Teams to adopt Breaking Changes. | ||
* Breaking Changes should be adopted within Breaking Change Window. |
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.
Regarding the pre-announce breaking change process part, I personally suggest that PM @dcaro and @Jacekey23 can also help review it when they have time after the PR is ready
### Overview | ||
|
||
* CLI Owned Module | ||
* Service Team should create an Issue that requests CLI Team to create the pre-announcement several sprints ahead Breaking Change Window. The CLI team will look at the issue and evaluate if it will be accepted in the next breaking change release. |
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.
Perhaps we can consider asking service team to add the tag of breaking-change request
to this type of issue, which can make it easier for us to pay attention to these important issues in the vast number of issues, it also facilitates us to conduct statistics and analysis through tag filtering in the future
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.
create an Issue
Additionally, it is recommended to provide a link of the corresponding feature request template here
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.
Perhaps we can consider asking service team to add the tag of
breaking-change request
to this type of issue
In the current Implementing Breaking Changes Doc, the created issue should be attached with Breaking Change
label. I think we could follow that design.
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.
like the tag of breaking-change, it's worth having it and align with services to start using.
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.
Add a sentence
The issue should include the label Breaking Change.
|
||
* CLI Owned Module | ||
* Service Team should create an Issue that requests CLI Team to create the pre-announcement several sprints ahead Breaking Change Window. The CLI team will look at the issue and evaluate if it will be accepted in the next breaking change release. | ||
* Please ensure sufficient time for CLI Team to finish the pre-announcement. |
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.
sufficient time
It would be better if more specific time suggestions could be provided
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.
maybe vote for sufficient time here?
4-week
6-week
10-week
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.
We can handle the pre-announce request in the same manner as other feature requests with a specific target date. For Breaking Change Pre-announce requests, the target date will be the release before the Breaking Change release.
|
||
### Breaking Change Window | ||
|
||
The breaking change window *allows* for service command breaking changes. When a Pull Request is merged during this sprint, it will be included in the next Breaking Change Release. |
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.
would be "this sprint" refer to any sprint that comes right before Build/Ignite? It's nice if be clear about it.
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.
Changed the sentence to
The breaking change window is a designated sprint that permits the merging of service command breaking changes. Any Pull Request merged during this sprint will be included in the subsequent Breaking Change Release.
|
||
The timing of the breaking change window in Azure CLI aligns with [Microsoft Build](https://build.microsoft.com/) and [Microsoft Ignite](https://ignite.microsoft.com/). You could find the next Breaking Change Release plan in our [milestones](https://github.com/Azure/azure-cli/milestones). | ||
|
||
> If you would like to release ad-hoc breaking changes, reach out to the CLI team to provide an explanation for the necessity of these changes. The exceptions can be provide in the following cases: |
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.
my 2 cents here:
I personally like to rephrase this part and tie with why/what we build breaking change window, mentioned in the top.
Making the request easy will likely get us difficult to hold the door.
It's highlighted that we usually do not allow breaking changes check-ins within non-breaking-change window, based on what we stated above for consistency and stable tooling experience.
something unexpected can happen. We understand and would like to help out. There would be high-graded justifications required to provide the info Azure CLI can access.
p.s.
Noting that providing the required info for assessment does not mean it will be assured to be greenlighted for breaking changes. Team will still make the decision based on the overall impact.
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.
I agree with that. I have replaced the parts about the ad-hoc introduction of breaking changes. Kindly review the modified content and provide your feedback.
### Overview | ||
|
||
* CLI Owned Module | ||
* Service Team should create an Issue that requests CLI Team to create the pre-announcement several sprints ahead Breaking Change Window. The CLI team will look at the issue and evaluate if it will be accepted in the next breaking change release. |
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.
like the tag of breaking-change, it's worth having it and align with services to start using.
|
||
* CLI Owned Module | ||
* Service Team should create an Issue that requests CLI Team to create the pre-announcement several sprints ahead Breaking Change Window. The CLI team will look at the issue and evaluate if it will be accepted in the next breaking change release. | ||
* Please ensure sufficient time for CLI Team to finish the pre-announcement. |
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.
maybe vote for sufficient time here?
4-week
6-week
10-week
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
|
||
The timing of the breaking change window in Azure CLI aligns with [Microsoft Build](https://build.microsoft.com/) and [Microsoft Ignite](https://ignite.microsoft.com/). You could find the next Breaking Change Release plan in our [milestones](https://github.com/Azure/azure-cli/milestones). | ||
|
||
> It's highlighted that the introduction of breaking changes is typically prohibited within non-breaking-change window, based on what we stated above for consistency and stable tooling experience. |
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.
> It's highlighted that the introduction of breaking changes is typically prohibited within non-breaking-change window, based on what we stated above for consistency and stable tooling experience. | |
> It's highlighted that the introduction of breaking changes is typically prohibited within non-breaking-change window, based on what we stated above for consistency and stable tooling experience. |
pass | ||
|
||
|
||
class Remove(BreakingChange): |
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.
a little concern about BreakingChange related class naming. It might be a little short and common, like Remove
, Rename
, and may cause name conflicts easily.
For me, adding a common prefix for these classes' name would be more clear and specific, like AzCLIRenameLog
, AzCLIRemoveLog
or something else? ( I add Log
here cause these classes seem to be used to log all kinds of breaking changes infos, you might have different considerations.)
Related command
Description
Introduce an additional method to pre-announce upcoming breaking changes other than deprecation. This will help with the collection of announced breaking changes.
Testing Guide
History Notes
[Component Name 1] BREAKING CHANGE:
az command a
: Make some customer-facing breaking change[Component Name 2]
az command b
: Add some customer-facing featureThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.