-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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 CognitiveServices.ComputerVision tests for full coverage #4390
Conversation
GitHub user: Azure | ||
Branch: current | ||
Commit: cb9c18b988dd67894653a34dc55b7978403b120a | ||
GitHub user: cthrash |
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.
Please generate code from the Azure master branch when the spec PR is merged
@@ -3,7 +3,7 @@ | |||
<PropertyGroup> | |||
<Description>Microsoft.Azure.CognitiveServices.Vision.ComputerVision.Tests Class Library</Description> | |||
<AssemblyName>Microsoft.Azure.CognitiveServices.Vision.ComputerVision.Tests</AssemblyName> | |||
<VersionPrefix>1.0.2-preview</VersionPrefix> |
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.
The test project is never published, we recommend setting the version to 1.0.0
and never change it.
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.
Done
@@ -6,7 +6,7 @@ | |||
<PropertyGroup> | |||
<PackageId>Microsoft.Azure.CognitiveServices.Vision.ComputerVision</PackageId> | |||
<Description>This client library provides access to the Microsoft Cognitive Services ComputerVision APIs.</Description> | |||
<VersionPrefix>1.0.2-preview</VersionPrefix> | |||
<VersionPrefix>1.0.3</VersionPrefix> | |||
<AssemblyName>Microsoft.Azure.CognitiveServices.Vision.ComputerVision</AssemblyName> | |||
<PackageTags>Microsoft Cognitive Services;Cognitive Services;Cognitive Services SDK;REST HTTP client;Computer Vision;Computer Vision API;Computer Vision SDK;Vision;netcore451511</PackageTags> | |||
<PackageReleaseNotes>This is a preview release of the Cognitive Services Computer Vision SDK.</PackageReleaseNotes> |
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.
Please update PackageReleaseNotes
Also, please rebase the commits to a single commit and update the PR |
* New SDK generated based on updated Swagger * New images added for test * New tests added for full coverage * Bump version to 1.0.3
1d5e175
to
ca5fdde
Compare
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.
Updates made based on comments
@@ -3,7 +3,7 @@ | |||
<PropertyGroup> | |||
<Description>Microsoft.Azure.CognitiveServices.Vision.ComputerVision.Tests Class Library</Description> | |||
<AssemblyName>Microsoft.Azure.CognitiveServices.Vision.ComputerVision.Tests</AssemblyName> | |||
<VersionPrefix>1.0.2-preview</VersionPrefix> |
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.
Done
bump |
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.
Looks good for the most part, needs a couple of comments addressed. Apologize for the turnaround
@@ -6,10 +6,10 @@ | |||
<PropertyGroup> | |||
<PackageId>Microsoft.Azure.CognitiveServices.Vision.ComputerVision</PackageId> | |||
<Description>This client library provides access to the Microsoft Cognitive Services ComputerVision APIs.</Description> | |||
<VersionPrefix>1.0.2-preview</VersionPrefix> | |||
<VersionPrefix>1.0.3</VersionPrefix> |
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.
Please confirm you plan on releasing a stable version here. If yes, versioning should start from 1.1.0
Also, all the projects need an AssemblyInfo.cs
file similar to this with appropriate package versions.
Thanks for the review. Version updated, metadata added. |
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.
LGTM
Will merge on CIs passing |
A number of bugs were discovered in the Swagger, and were fixed in this PR.
Description
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
Testing Guidelines
SDK Generation Guidelines
*.csproj
andAssemblyInfo.cs
files have been updated with the new version of the SDK.