Skip to content

Introducing Security Center .Net SDK #4548

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.

Sign up for GitHub

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

Jump to bottom
Merged
dsgouda merged 1 commit into from
Jul 27, 2018
Merged

Introducing Security Center .Net SDK #4548

dsgouda merged 1 commit into from
Jul 27, 2018

Conversation

@chlahav
Copy link
Contributor

@chlahav chlahav commented Jul 16, 2018

Description

Introducing Security Center .Net SDK from a new swagger spec

First swagger PR

Spec PR
Spec PR
Spec PR
Spec PR
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.

@chlahav chlahav force-pushed the psSdkJson6 branch 2 times, most recently from 3d8be71 to 9d51179 Compare July 17, 2018 06:40
@dsgouda
Copy link
Contributor

dsgouda commented Jul 20, 2018

@chlahav Please update the PR with latest upstream changes

@chlahav chlahav force-pushed the psSdkJson6 branch 3 times, most recently from a83349d to 76cdee0 Compare July 22, 2018 13:50
@chlahav
Copy link
Contributor Author

chlahav commented Jul 22, 2018

@dsgouda PR was updated with the latest changes

dsgouda
dsgouda suggested changes Jul 23, 2018
View reviewed changes
Copy link
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 for the most part. Please address the comments posted.

...Ks/SecurityCenter/Management.SecurityCenter/Microsoft.Azure.Management.SecurityCenter.csproj Outdated
Copy link
Contributor

@dsgouda dsgouda Jul 23, 2018

Choose a reason for hiding this comment

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

Looks like the SDK is still in preview version. The Nuget package must also be in preview. Preview versioning must start from
0.9.0-preview

Copy link
Contributor Author

@chlahav chlahav Jul 24, 2018

Choose a reason for hiding this comment

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

changed to 0.9.0-preview

src/SDKs/SecurityCenter/Management.SecurityCenter/generate.ps1 Outdated
Copy link
Contributor

@dsgouda dsgouda Jul 23, 2018

Choose a reason for hiding this comment

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

Please confirm that the Sdk was generated using this script.
There must also be a SdkInfo_*.cs file and a .txt log file generated, please commit these files

Copy link
Contributor Author

@chlahav chlahav Jul 24, 2018

Choose a reason for hiding this comment

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

the generated files were generated using this script.
I also added the SdkInfo file and the text file

@chlahav chlahav force-pushed the psSdkJson6 branch 2 times, most recently from 3c09418 to 3bb6c62 Compare July 24, 2018 18:47
shahabhijeet
shahabhijeet suggested changes Jul 24, 2018
View reviewed changes
src/SDKs/SecurityCenter/Management.SecurityCenter/Properties/AssemblyInfo.cs Outdated
Copy link
Contributor

@shahabhijeet shahabhijeet Jul 24, 2018

Choose a reason for hiding this comment

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

@chlahav update assemblyversion/fileversion to 0.9.0.0

Copy link
Contributor Author

@chlahav chlahav Jul 24, 2018

Choose a reason for hiding this comment

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

@shahabhijeet done

dsgouda
dsgouda suggested changes Jul 25, 2018
View reviewed changes
Copy link
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, just address the one comment and update the branch and we should be good

...Ks/SecurityCenter/Management.SecurityCenter/Microsoft.Azure.Management.SecurityCenter.csproj Outdated
Copy link
Contributor

@dsgouda dsgouda Jul 25, 2018

Choose a reason for hiding this comment

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

public preview release

@chlahav chlahav force-pushed the psSdkJson6 branch 2 times, most recently from 02a2e94 to a08be95 Compare July 25, 2018 18:52
dsgouda
dsgouda approved these changes Jul 25, 2018
View reviewed changes
Copy link
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 subject to CIs passing

@chlahav
Copy link
Contributor Author

chlahav commented Jul 26, 2018

@dsgouda All the CI tests passed, if there are no additional comments, can you please merge the CR?
thanks!

@dsgouda
Copy link
Contributor

dsgouda commented Jul 26, 2018

@chlahav Please update the branch with upstream psSdkJson6

@chlahav
Copy link
Contributor Author

chlahav commented Jul 26, 2018

@dsgouda branch was updated and all the tests passed

@dsgouda dsgouda merged commit 84237d0 into Azure:psSdkJson6 Jul 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@dsgouda dsgouda dsgouda approved these changes

@shahabhijeet shahabhijeet shahabhijeet 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.

3 participants

@chlahav @dsgouda @shahabhijeet