Skip to content

Adding container registry dataplane sdk#4856

Merged
dsgouda merged 5 commits intoAzure:psSdkJson6from
Jinhuafei:acr_dataplane
Oct 16, 2018
Merged

Adding container registry dataplane sdk#4856
dsgouda merged 5 commits intoAzure:psSdkJson6from
Jinhuafei:acr_dataplane

Conversation

@Jinhuafei
Copy link
Copy Markdown
Contributor

@Jinhuafei Jinhuafei commented Oct 11, 2018

Description

Adding Azure container registry dataplane SDK for managing

  • repository
  • tag
  • manifest

The SDK is generated by this REST spec PR.

This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

SDK Generation Guidelines

  • If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
  • The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as the commitid of your swagger spec or link to the swagger spec, used to generate the code.
  • The *.csproj and AssemblyInfo.cs files have been updated with the new version of the SDK.

@Jinhuafei
Copy link
Copy Markdown
Contributor Author

Jinhuafei commented Oct 11, 2018 •
edited
Loading

/CC @sajayantony

dsgouda
dsgouda suggested changes Oct 11, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@dsgouda dsgouda left a comment

Choose a reason for hiding this comment

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

when code is generated using generate.ps1, an SdkInfo_*.cs file must be generated, please commit this file.

Comment thread ...plane/Microsoft.Azure.ContainerRegistry.Tests/Microsoft.Azure.ContainerRegistry.Tests.csproj Outdated
Comment thread ...plane/Microsoft.Azure.ContainerRegistry.Tests/Microsoft.Azure.ContainerRegistry.Tests.csproj Outdated
Comment thread src/SDKs/ContainerRegistry/dataplane/Microsoft.Azure.ContainerRegistry/generate.ps1 Outdated
Comment thread ...egistry/dataplane/Microsoft.Azure.ContainerRegistry/Microsoft.Azure.ContainerRegistry.csproj Outdated
Comment thread ...egistry/dataplane/Microsoft.Azure.ContainerRegistry/Microsoft.Azure.ContainerRegistry.csproj Outdated
dsgouda
dsgouda reviewed Oct 11, 2018
View reviewed changes
Comment thread ...plane/Microsoft.Azure.ContainerRegistry.Tests/Microsoft.Azure.ContainerRegistry.Tests.csproj
<PackageId>Microsoft.Azure.ContainerRegistry.Tests</PackageId>
<Description>Microsoft.Azure.ContainerRegistry tests Library</Description>
<AssemblyName>Microsoft.Azure.ContainerRegistry.Tests</AssemblyName>
<Version>1.0.0</Version>
Copy link
Copy Markdown
Contributor

@dsgouda dsgouda Oct 11, 2018

Choose a reason for hiding this comment

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

Please add <TreatWarningsAsErrors>true</TreatWarningsAsErrors> and fix all errors

dsgouda
dsgouda suggested changes Oct 11, 2018
View reviewed changes
Comment thread ...stry/dataplane/Microsoft.Azure.ContainerRegistry/Generated/SdkInfo_AzureContainerRegistry.cs
Comment thread ...egistry/dataplane/Microsoft.Azure.ContainerRegistry/Microsoft.Azure.ContainerRegistry.csproj Outdated
dsgouda
dsgouda suggested changes Oct 12, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@dsgouda dsgouda left a comment

Choose a reason for hiding this comment

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

Looks great, just a couple of comments

Comment thread src/SDKs/ContainerRegistry/dataplane/ContainerRegistry.Tests/ContainerRegistry.Tests.csproj Outdated
Comment thread ...egistry/dataplane/Microsoft.Azure.ContainerRegistry/Microsoft.Azure.ContainerRegistry.csproj Outdated
@Jinhuafei
Copy link
Copy Markdown
Contributor Author

Jinhuafei commented Oct 16, 2018

@dsgouda When will PR be merged and when will the nupkg be released?

dsgouda
dsgouda suggested changes Oct 16, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@dsgouda dsgouda 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 for the most part, apologize for the delay.

Comment thread ...egistry/dataplane/Microsoft.Azure.ContainerRegistry/Microsoft.Azure.ContainerRegistry.csproj
<Version>0.9.0-preview</Version>
<AssemblyName>Microsoft.Azure.ContainerRegistry</AssemblyName>
<PackageTags>Microsoft Azure Container Registry;Container Registry;</PackageTags>
<PackageReleaseNotes>Added capabilities to manage repository, tag and manifest.</PackageReleaseNotes>
Copy link
Copy Markdown
Contributor

@dsgouda dsgouda Oct 16, 2018

Choose a reason for hiding this comment

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

This should be release notes about the SDK being published rather than the commit.
For example

This is a public preview SDK for Microsoft.Azure.ContainerRegistry services

Copy link
Copy Markdown
Contributor Author

@Jinhuafei Jinhuafei Oct 16, 2018

Choose a reason for hiding this comment

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

Shouldn't the release notes list the change included in this release? The developers already know this is preview sdk through the package versioning.

Copy link
Copy Markdown
Contributor

@dsgouda dsgouda Oct 16, 2018

Choose a reason for hiding this comment

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

Ohh, I guess I understood it incorrectly, this SDK allows one to manage a repo.

dsgouda
dsgouda approved these changes Oct 16, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@dsgouda dsgouda left a comment

Choose a reason for hiding this comment

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

LGTM

@dsgouda dsgouda merged commit ae3e173 into Azure:psSdkJson6 Oct 16, 2018
@snyk-bot snyk-bot mentioned this pull request Oct 19, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@dsgouda dsgouda dsgouda approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Jinhuafei @dsgouda