Skip to content

Conversation

@JingchuanHuangMSFT
Copy link
Contributor

@JingchuanHuangMSFT JingchuanHuangMSFT commented Feb 18, 2025

Description

  • Pipeline Group upgraded API version to 2024-10-01-preview

Mandatory Checklist

  • SHOULD update ChangeLog.md file(s) appropriately
    • For SDK-based development mode, update src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.
      • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header in the past tense.
    • For autorest-based development mode, include the changelog in the PR description.
    • Should not change ChangeLog.md if no new release is required, such as fixing test case only.
  • SHOULD regenerate markdown help files if there is cmdlet API change. Instruction
  • SHOULD have proper test coverage for changes in pull request.
  • SHOULD NOT adjust version of module manually in pull request

@azure-client-tools-bot-prd
Copy link

Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status.

@NoriZC
Copy link
Contributor

NoriZC commented Feb 18, 2025

@JingchuanHuangMSFT Please follow our migration guide to develop autorest based module: start new development and have prior experience

@NoriZC NoriZC self-assigned this Feb 18, 2025
@github-actions
Copy link

This PR was labeled "needs-revision" because it has unresolved review comments or CI failures.
Please resolve all open review comments and make sure all CI checks are green. Refer to our guide to troubleshoot common CI failures.

@notyashhh
Copy link
Member

Hi @JingchuanHuangMSFT, if you are planning this change for the next release, it still needs to be revised according to this doc: New Guide

@JingchuanHuangMSFT
Copy link
Contributor Author

Thanks @notyashhh , I'm checking that and will start doing the necessary work this week.

@JingchuanHuangMSFT
Copy link
Contributor Author

@notyashhh I followed the guide to add the required files, could you review if the latest revision is okay?

@notyashhh notyashhh assigned notyashhh and unassigned NoriZC Mar 12, 2025
@notyashhh
Copy link
Member

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@JingchuanHuangMSFT
Copy link
Contributor Author

The build failure was caused by removal of a previously automatically generated property, breaking backwards compatibility static analysis check. I pushed a commit to add those properties back manually.

Reproduced this problem locally and confirmed fix: artifacts\StaticAnalysis\StaticAnalysis.Netcore.exe -p artifacts\Debug -r artifacts\StaticAnalysisResults --analyzers breaking-change --modules-to-analyze Az.Monitor

@notyashhh
Copy link
Member

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Member

@notyashhh notyashhh left a comment

Choose a reason for hiding this comment

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

I see the src/Monitor is updated to latest api version but not the Autorest part of the module. Can you confirm that this won't cause any breaking between any real-world scenarios?

Ignore this comment, I misunderstood the change.

@JingchuanHuangMSFT
Copy link
Contributor Author

@JingchuanHuangMSFT

I see the src/Monitor is updated to latest api version but not the Autorest part of the module. Can you confirm that this won't cause any breaking between any real-world scenarios?

I think I only intended to update src/Monitor/PipelineGroup.Autorest/README.md and its related files, could you elaborate what other files are under Autorest part of the module?

@notyashhh
Copy link
Member

Why is this PR marked as "public preview"? We are aiming for Az13.4.0 release correct?

@notyashhh
Copy link
Member

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@JingchuanHuangMSFT
Copy link
Contributor Author

Why is this PR marked as "public preview"? We are aiming for Az13.4.0 release correct?

The service and API version themselves are still in public preview state, so I think choosing public preview for powershell module is more appropriate. Do you think it's better to select general release here?

@notyashhh
Copy link
Member

@JingchuanHuangMSFT, Yes, this is classified as a general release even if the api version is in public-preview, Our "public-preview" option is only used for modules that maintain a series of Az module preview versions.

And since this will go in general release, it will be released as 1 minor version upgrade. 😄

@JingchuanHuangMSFT
Copy link
Contributor Author

@JingchuanHuangMSFT, Yes, this is classified as a general release even if the api version is in public-preview, Our "public-preview" option is only used for modules that maintain a series of Az module preview versions.

And since this will go in general release, it will be released as 1 minor version upgrade. 😄

Got it, I've updated the PR description.

@JingchuanHuangMSFT
Copy link
Contributor Author

@notyashhh Are there any remaining required actions for me to get this PR merged? Thanks.

@notyashhh notyashhh added this to the Az 13.4.0 (04/01/2025) milestone Mar 19, 2025
Copy link
Member

@notyashhh notyashhh left a comment

Choose a reason for hiding this comment

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

Otherwise looks great!

@JingchuanHuangMSFT
Copy link
Contributor Author

Updated.

@notyashhh
Copy link
Member

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Member

@notyashhh notyashhh 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!

@notyashhh notyashhh merged commit 227a9f8 into Azure:main Mar 20, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants