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

Enable Change log check during release and analyse step #12566

Closed
praveenkuttappan opened this issue Jul 16, 2020 · 7 comments · Fixed by #15274
Closed

Enable Change log check during release and analyse step #12566

praveenkuttappan opened this issue Jul 16, 2020 · 7 comments · Fixed by #15274
Assignees
Labels
Central-EngSys This issue is owned by the Engineering System team. EngSys This issue is impacting the engineering system.

Comments

@praveenkuttappan
Copy link
Member

Change log check is disabled now due to issues we have noticed during last release. We need to make required fix and enable this before next release. Here are the two issues we have noticed last.

  1. Preview release version is incorrectly parsed or formatted for no change log is found for the version
  2. Unreleased change log header check should be done only for release build and not for manual build. Both analyze and release step should perform same set of checks.

One solution for issue #2 is as follows:

  1. Create a new build time variable "Build.ForRelease" and this should be defaulted to true for any manually queued build. If build.forRelease is true then we should perform same set of checks during release and analyze step.
  2. When build is queued manually and for not release purpose then this variable needs to be explicitly set to false.

@scbedd Please let me know if I have missed any point.

@chidozieononiwu : just FYI.

@praveenkuttappan praveenkuttappan added the EngSys This issue is impacting the engineering system. label Jul 16, 2020
@praveenkuttappan praveenkuttappan self-assigned this Jul 16, 2020
@chidozieononiwu
Copy link
Member

  1. The issue with preview version being formatted wrongly is fixed with https://github.com/Azure/azure-sdk-tools/pull/772/files

  2. Why is this necessary? The intention is to run the Verify-ChangeLog on release only when there is an actual release approval. In which case you are already certain it is going out for release. in which case all you need to do is pass the ForRelease parameter when you call the template. Unless there is some other issue you intend to avoid I believe this should be sufficient.

@scbedd
Copy link
Member

scbedd commented Jul 16, 2020

Why is this necessary? The intention is to run the Verify-ChangeLog on release only when there is an actual release approval.

You're absolutely correct, but we have changelog verification in our BUILD phase as well. What's currently happening is that the "Build" stage is SUCCEEDING prior to failing during the release. This is super confusing for the devs because the build stage (which INCLUDES verify-changelog) WAS GREEN!

@praveenkuttappan
Copy link
Member Author

Our intention is to ensure that same check is performed during analyze and release for release build so if release is going to find any issue then it's better to find that in analyze phase

@chidozieononiwu
Copy link
Member

chidozieononiwu commented Jul 16, 2020

Ok, I see what you mean. I will add the check to the verify-changelog.yml.

@chidozieononiwu
Copy link
Member

I have a PR addressing this Azure/azure-sdk-tools#797

@weshaggard
Copy link
Member

While I completely understand there is a little confusion between running the verify-changelog step as part of the build and as part of the release there are reasons for splitting the verification. We want to do as much verification as we can during the build/analyze phase to make sure folks are thinking about and at least have a place holder for the current active version. During release we want to verify that the change log is not empty and that the version entry is not marked as unreleased. We cannot do this step during the build/analyze step because we don't know if they plan to release a given package. Imagine a pipeline that has multiple packages and only a subset of packages are planning to release we cannot enforce the release criteria on the other packages.

So in our current system I don't think we can move all the validation into the build/analyze step. If we feel having it run it both build and release adds to confusion we could move all the validation to the release stage for a build we plan to do a release from, but we still want the verification in the build/analyze phase the rest of the time so people are setup for success during development.

@weshaggard
Copy link
Member

Reopening this until we fix the python issues as we reverted part of this in #15356

@weshaggard weshaggard reopened this Nov 17, 2020
openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-python that referenced this issue Jan 28, 2021
Update quantum python track2 config (Azure#12566)

* update quantum python track2 config

* Update readme.python.md

Co-authored-by: msyyc <[email protected]>
openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-python that referenced this issue Jun 3, 2021
Update quantum python track2 config (Azure#12566)

* update quantum python track2 config

* Update readme.python.md

Co-authored-by: msyyc <[email protected]>
msyyc added a commit that referenced this issue Aug 4, 2021
* CodeGen from PR 12566 in Azure/azure-rest-api-specs
Update quantum python track2 config (#12566)

* update quantum python track2 config

* Update readme.python.md

Co-authored-by: msyyc <[email protected]>

* version,CHANGELOG

* Update CHANGELOG.md

* Update CHANGELOG.md

Co-authored-by: SDKAuto <[email protected]>
Co-authored-by: msyyc <[email protected]>
Co-authored-by: PythonSdkPipelines <PythonSdkPipelines>
Co-authored-by: Zed Lei <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Central-EngSys This issue is owned by the Engineering System team. EngSys This issue is impacting the engineering system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants