Skip to content

Cognitive Services version 2017-04-18#3221

Merged
shahabhijeet merged 13 commits intoAzure:psSdkJson6from
felixwa:Version2017-04-18
May 26, 2017
Merged

Cognitive Services version 2017-04-18#3221
shahabhijeet merged 13 commits intoAzure:psSdkJson6from
felixwa:Version2017-04-18

Conversation

@felixwa
Copy link
Contributor

@felixwa felixwa commented May 16, 2017

Generates SDK against Cognitive Services version 2017-04-18 swagger
file.
Fixes failed test cases accrodingly and adds 2 more test cases.
Bump version to 1.0.0.0 as Cognitive Services has been GA.

Description


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.

Generates SDK against Cognitive Services version 2017-04-18 swagger
file.
Fixes failed test cases accrodingly and adds 2 more test cases.
Bump version to 1.0.0.0 as Cognitive Services has been GA.
Bump VersionPrefix in csproj to 1.0.0
@felixwa
Copy link
Contributor Author

felixwa commented May 16, 2017

var rgname = CognitiveServicesManagementTestUtilities.CreateResourceGroup(resourcesClient);

var accounts = cognitiveServicesMgmtClient.CognitiveServicesAccounts.ListByResourceGroup(rgname);
var accounts = cognitiveServicesMgmtClient.Accounts.ListByResourceGroup(rgname);
Copy link
Contributor

Choose a reason for hiding this comment

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

@felixwa I see you are still using really old ResourceManger nuget package
Please use the latest nugets for all the dependencies that are used in the tests.
We are asking every RP to update their tests that uses latest nuget packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shahabhijeet I have updated all nuget packages for test project though for ResourceManager (Microsoft.Azure.ResourceManager) I think we have used the latest (1.1.0-preview). I don't see update option for it.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, so actually it is a NEW NuGet package. Added.

<PropertyGroup>
<Description>Microsoft Azure Management Cognitive Services Library</Description>
<VersionPrefix>0.3.0-preview</VersionPrefix>
<VersionPrefix>1.0.0</VersionPrefix>
Copy link
Contributor

Choose a reason for hiding this comment

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

@felixwa please replace the entire csproj with the following

Microsoft.Azure.Management.CognitiveServices Microsoft Azure Management Cognitive Services Library 1.0.0 Microsoft.Azure.Management.CognitiveServices Microsoft Azure Cognitive Services management;Cognitive Services;Cognitive Services management;REST HTTP client;windowsazureofficial;netcore451511 net452;netstandard1.4

If you want to add ReadMe.txt please add .nuspec file, here is an example
https://docs.microsoft.com/en-us/nuget/schema/nuspec#including-content-files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated accordingly.
I will add that additional notice in another PR.

felixwa added 3 commits May 18, 2017 12:19
Cleans up .csproj file as shahabhijeet suggested.
Fix format of csproj
Update NuGet pacakges in test project to the latest.
@shahabhijeet shahabhijeet changed the base branch from vs17Dev to psSdkJson6 May 22, 2017 20:13
Copy link
Contributor

@shahabhijeet shahabhijeet left a comment

Choose a reason for hiding this comment

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

Once you are done making suggested changes, remove "needs-revision" flag and add "Need review" flag

<Service Include="{82a7f48d-3b50-4b1e-b82e-3ada8210c358}" />
</ItemGroup>

<ItemGroup>
Copy link
Contributor

Choose a reason for hiding this comment

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

@felixwa any reason these references are added?
All these references are already implicitly added to your project, you do not need them again.
Please remove, or let me know the reason you made this change?

Copy link
Contributor

@shahabhijeet shahabhijeet May 23, 2017

Choose a reason for hiding this comment

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

@felixwa in order to use latest ResourceManagement, you will have to add reference to the latest resource manager nuget 1.6.0-preview, and make that change inside CognitiveServicesManagementTestUtilities.cs (that creates the ResourceManagementClient).
We ask RP to re record all tests, that is idea for two reasons:

  1. ResourceManager 1.1.0-preview is really old and if you are still using it, you are not testing latest that is being used by your customers that are creating cognitive services resources.
  2. If you cannot re-record all tests, then start with recording tests that are being used by this particular PR.

Also please remove Compiler Constant #DNX451 from your test/SDK project.
The only constant you should be using is
#if FullNetFx (for Desktop framework version e.g. .NET 4.5.2 .NET 4.6 etc) and if you are targeting .NET Standard, please use
#if !FullNetFx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shahabhijeet,

  1. Removed those references. They were automatically added when I updated them in NuGet manager.
  2. Add Microsoft.Azure.Management.ResourceManager 1.6.0-preview reference and changed UT code a little bit accordingly.
  3. Re-recorded all unit tests.
  4. Removed that #DNX451 constant.

felixwa added 7 commits May 26, 2017 18:06
Generates SDK against Cognitive Services version 2017-04-18 swagger
file.
Fixes failed test cases accrodingly and adds 2 more test cases.
Bump version to 1.0.0.0 as Cognitive Services has been GA.
Bump VersionPrefix in csproj to 1.0.0
Cleans up .csproj file as shahabhijeet suggested.
Fix format of csproj
Update NuGet pacakges in test project to the latest.
Remove DNX451 symbol
@felixwa felixwa force-pushed the Version2017-04-18 branch from dae445a to ccebabf Compare May 26, 2017 10:28
1. Adds ref to Microsoft.Azure.Management.ResourceManager 1.6.0-preview
2. Adjust some test code accordingly.
3. Re-record all unit tests.
@shahabhijeet shahabhijeet merged commit 9ce9125 into Azure:psSdkJson6 May 26, 2017
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.

5 participants

Comments