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

Add template script #15272

Closed
wants to merge 2 commits into from
Closed

Conversation

ArcturusZhang
Copy link
Member

@ArcturusZhang ArcturusZhang commented Aug 12, 2021

This PR adds a template for mgmt plane packages and introduces a script to apply the template with the given parameters.

  • The purpose of this PR is explained in this or a referenced issue.
  • The PR does not update generated files.
  • Tests are included and/or updated for code changes.
  • Updates to CHANGELOG.md are included.
  • MIT license headers are included in each file.

@check-enforcer
Copy link

This repository is protected by Check Enforcer. The check-enforcer check-run will not pass until there is at least one more check-run successfully passing. Check Enforcer supports the following comment commands:

  • /check-enforcer evaluate; tells Check Enforcer to evaluate this pull request.
  • /check-enforcer override; by-pass Check Enforcer (approvals still required).

@ArcturusZhang ArcturusZhang marked this pull request as ready for review August 16, 2021 05:02
@ArcturusZhang ArcturusZhang requested review from benbp and a team as code owners August 16, 2021 05:02
@@ -0,0 +1,3 @@
# Release History

## v0.1.0 (released)
Copy link
Member

Choose a reason for hiding this comment

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

Can you help me understand why this is 0.1.0? In all other languages we start at 1.0.0-beta.1 and unless there is a strong reason I'd suggest we do that in go as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

This might be the golang way to represent a pre-release before v1.0.0 - but I would like to defer @jhendrixMSFT to provide some more details with more accuracy.

Copy link
Member

Choose a reason for hiding this comment

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

While golang does support a "0.x.x" versioning scheme so does a lot of other languages but we have been trying to not use that in other places as it tends to make people believe we are a far away from anything they can depend on. golang does support the prerelease versioning scheme as well according to https://golang.org/doc/modules/version-numbers#pre-release so unless there is strong reason we should be consistent with our versioning polices and go with 1.0.0-beta.1 as the first version (https://azure.github.io/azure-sdk/policies_releases.html). @jhendrixMSFT I know we talked about this a little in the past but I'd like to revisit this given we are going to be having a large influx of new libraries.

@@ -0,0 +1,76 @@
# Azure {{PackageTitle}} Module for Go

[![PkgGoDev](https://pkg.go.dev/badge/github.com/Azure/azure-sdk-for-go/sdk/{{rpName}}/{{packageName}})](https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/{{rpName}}/{{packageName}})
Copy link
Member

Choose a reason for hiding this comment

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

Based on our other conversations this will always end up being a broken link for new libraries. Have we came up with a strategy for that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately no. We do not really want to ship this without the link - this will be the link to package references

Copy link
Member

Choose a reason for hiding this comment

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

We at least need to pick a strategy for folks because this will break in CI for every new library we use it on.

Copy link
Member Author

@ArcturusZhang ArcturusZhang Aug 23, 2021

Choose a reason for hiding this comment

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

I once made a change to the ci.yml template to make sure we could go over every step in the build analysis job by moving the link verification to the last step). In this way, when we are onboarding a new RP, we can ensure that if the only failure in this PR is the link verification, and the RP is generated from a specific template, this PR should be fine and the invalid link will be good after the PR is merged and the tag is added. This is only current practical way to do service onboarding.
What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I suppose that is one option but it does mean that you will have to override any PR with changes to that file before it is release the first time. It isn't ideal and if we get the link wrong or things change/rename before we release we risk shipping a broken link here.

I personally like leaving this commented out by default and after our first release we can uncomment it.


``` yaml
require:
- https://github.com/Azure/azure-rest-api-specs/blob/{{commitID}}/specification/{{rpName}}/resource-manager/readme.md
Copy link
Member

Choose a reason for hiding this comment

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

Will these links actually exist initially?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, these must exist, otherwise the autorest command will complain it cannot get these files.

@@ -0,0 +1,15 @@
# NOTE: Please refer to https://aka.ms/azsdk/engsys/ci-yaml before editing this file.
trigger:
Copy link
Member

Choose a reason for hiding this comment

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

Lets be sure to add the branch names to these triggers as well.

@weshaggard
Copy link
Member

I see that this assumes mgmt libraries how much would it take to also add support for data-plane libraries as well?

cc @RickWinter @jhendrixMSFT

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.

2 participants