Skip to content

Namespace change for SpellCheck C# SDK#4219

Merged
dsgouda merged 15 commits intoAzure:psSdkJson6from
ashku-ms:NamespaceChange
Apr 19, 2018
Merged

Namespace change for SpellCheck C# SDK#4219
dsgouda merged 15 commits intoAzure:psSdkJson6from
ashku-ms:NamespaceChange

Conversation

@ashku-ms
Copy link
Contributor

Description

Keeping in line with the standardization efforts, changing the namespace of C# SDK to the prescribed format Microsoft.Azure.CognitiveServices. The spec was reviewed here

Azure/azure-rest-api-specs#2859


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.

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.

Needs version update.

<Description>This client library provides access to the Microsoft Cognitive Services SpellCheck API.</Description>
<VersionPrefix>1.2.0</VersionPrefix>
<AssemblyName>Microsoft.Azure.CognitiveServices.SpellCheck</AssemblyName>
<VersionPrefix>1.3.0-preview</VersionPrefix>
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing namespace name is a breaking change. You might want to rev the major version number (2.0.0)
@shahabhijeet FYI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


[assembly: AssemblyVersion("1.0.0.0")]
[assembly: AssemblyFileVersion("1.2.0.0")]
[assembly: AssemblyFileVersion("1.3.0.0")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update version numbers here too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -8,12 +8,12 @@
[assembly: AssemblyDescription("Provides API functions for consuming the Microsoft Azure Cognitive Services SpellCheck API.")]

[assembly: AssemblyVersion("1.0.0.0")]
Copy link
Contributor

Choose a reason for hiding this comment

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

In case of major version update, AssemblyVersion must also be updated. Apologize for the back and forth

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nw done :)

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.

  • Please create a generate.ps1 file similar to this and regenerate the code
  • Please run
    msbuild build.proj /t:build /p:Scope=SDKs/CognitiveServices/dataPlane/Language/SpellCheck/BingSpellCheck
    Please commit any artifacts generated by these commits.

<AssemblyName>Microsoft.Azure.CognitiveServices.Language.SpellCheck</AssemblyName>
<PackageTags>Microsoft Cognitive Services;Cognitive Services;Cognitive Services SDK;SpellCheck API;REST HTTPclient;SpellCheckSDK;netcore451511</PackageTags>
<PackageReleaseNotes>This is a public release of the Cognitive Services SpellCheck SDK. Included with this release is support for SpellCheck API.</PackageReleaseNotes>
<PackageReleaseNotes>This is a preview release of the Cognitive Services SpellCheck SDK. Included with this release is support for SpellCheck API.</PackageReleaseNotes>
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is a preview, the package should be versioned as 2.0.0-preview

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the generate.cmd file in this directory. Does msbuild build.proj generate the files ? how I run this exactly ? where do i find msbuild and build.proj ?

@dsgouda
Copy link
Contributor

dsgouda commented Apr 17, 2018

@ashku-ms
Please regenerate this code using generate.cmd/generate.ps1 and commit all the files modified/generated.

@@ -0,0 +1 @@
powershell.exe -ExecutionPolicy Bypass -NoLogo -NonInteractive -NoProfile -File "..\..\..\..\..\..\..\tools\generateTool.ps1" -ResourceProvider "cognitiveservices\data-plane\SpellCheck" -PowershellInvoker -AutoRestVersion "latest" No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace this with
powershell.exe -ExecutionPolicy Bypass -NoLogo -NonInteractive -NoProfile -File "..\..\..\..\..\..\..\tools\generateTool.ps1" -ResourceProvider "cognitiveservices/data-plane/SpellCheck/BingSpellCheck" -PowershellInvoker -AutoRestVersion "latest"

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

@dsgouda dsgouda merged commit e6ce594 into Azure:psSdkJson6 Apr 19, 2018
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.

3 participants