-
Notifications
You must be signed in to change notification settings - Fork 848
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: mgmt plane release tool update #16238
Conversation
tadelesh
commented
Nov 22, 2021
•
edited
Loading
edited
- The purpose of this PR is explained in this or a referenced issue.
- The PR does not update generated files.
- These files are managed by the codegen framework at Azure/autorest.go.
- Tests are included and/or updated for code changes.
- Updates to CHANGELOG.md are included.
- MIT license headers are included in each file.
- change second generate cmd to version replacement for update release
- refine changelog file content set based on new changelog format
- add refresh-v2 cmd for convenience
- add package config support for muliti-package rp onboard
…elog file content set based on new changelog format
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.
Some minor comments
} | ||
|
||
log.Printf("Release generation for rp: %s, namespace: %s", c.rpName, c.namespaceName) | ||
generateCtx := common.GenerateContext{ | ||
SDKPath: sdkRepo.Root(), | ||
SDKRepo: &sdkRepo, | ||
SDKPath: (*sdkRepo).Root(), |
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.
Why we have to dereference this variable? this looks weird
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.
commitIDRegex = regexp.MustCompile("^[0-9a-f]{40}$") | ||
) | ||
|
||
func GetSDKRepo(sdkRepoParam, sdkRepoURL string) (*repo.SDKRepository, error) { |
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.
If I did not remember wrong, repo.SDKRepository
is an interface, therefore we did not need to return pointer 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.
Changed.
@@ -9,4 +9,5 @@ require: | |||
- https://github.com/Azure/azure-rest-api-specs/blob/{{commitID}}/specification/{{rpName}}/resource-manager/readme.go.md | |||
license-header: MICROSOFT_MIT_NO_VERSION | |||
module-version: 0.1.0 | |||
{{packageConfig}} |
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.
What does this mean?
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 have added a config param to release-v2 for multi-package RP's onboard which can be used to add config like package-singleservers: true
. Also, package config will be extracted from swagger config yaml annotation for SDK automation onboard.
* feat: change second generate cmd to version replacement, refine changelog file content set based on new changelog format * feat: add refresh-v2 cmd * fix: add package config support for muliti-package rp onboard