Skip to content

first version of C# apim spec on autorest#3544

Merged
shahabhijeet merged 2 commits intoAzure:psSdkJson6from
solankisamir:psSdkJson6
Aug 10, 2017
Merged

first version of C# apim spec on autorest#3544
shahabhijeet merged 2 commits intoAzure:psSdkJson6from
solankisamir:psSdkJson6

Conversation

@solankisamir
Copy link
Copy Markdown
Member

@solankisamir solankisamir commented Aug 3, 2017

Description

This is the first version of the API Management C# client library based on AutoRest spec.


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.

@msftclas
Copy link
Copy Markdown

msftclas commented Aug 3, 2017

@solankisamir,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@solankisamir solankisamir assigned KedarJoshi and unassigned KedarJoshi Aug 3, 2017
@solankisamir solankisamir requested review from KedarJoshi and cormacpayne and removed request for olydis August 3, 2017 03:23
Copy link
Copy Markdown
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.

@solankisamir can you provide link to the Swagger PR that was merged.

<PropertyGroup>
<PackageId>ApiManagement.Tests</PackageId>
<Description>ApiManagement.Tests Class Library</Description>
<VersionPrefix>1.0.0-preview</VersionPrefix>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@solankisamir make versionPrefix as version and remove version tag from below property group.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

#fixed

<Folder Include="Resources\" />
</ItemGroup>
<ItemGroup>
<None Update="Resources\SwaggerPetStoreV2.json">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@solankisamir remove exclusive enumeration of files, rather do exactly what have been done in the above itemgroup for all *.json files under SessionRecords.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

#fixed

{
HttpMockServer.Variables[TestCertificateKey] = base64EncodedTestCertificateData;
}
this.testCertificatePassword = testEnv.ConnectionString.KeyValuePairs.GetValueUsingCaseInsensitiveKey(TestCertificatePasswordKey);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@solankisamir Can you not use GetValueusingCaseInsenitiveKey API, I am planning to deprecate that function as KeyVaulePair Dictionary has been implemented to be case Insensitve.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

#fixed

}
else if (HttpMockServer.Mode == HttpRecorderMode.Playback)
{
this.tenantId = testEnv.Tenant;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@solankisamir maybe it's not clear to me, but why you need to do all this initialization during playback?

Copy link
Copy Markdown
Member Author

@solankisamir solankisamir Aug 6, 2017

Choose a reason for hiding this comment

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

remove some extra variables. I require some initialization during playback, as some of the tests can only be executed on an active service. Service Activation takes 40 minutes, hence I use this as a place to provide active service details and proceed with additional validation.

{
public static class Extensions
{
public static bool DictionaryEqual<TKey, TValue>(this IDictionary<TKey, TValue> first, IDictionary<TKey, TValue> second)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@solankisamir
nit: Can you not use Linq extension methods first.Except(second) or better use Linq to do the comparison.
I assume you only want to compare keys and not their values, in that case you can still do that using the above

<TargetFrameworks>net452;netstandard1.4</TargetFrameworks>
</PropertyGroup>
<ItemGroup>
<Folder Include="Customization\Models\" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@solankisamir you don't need to include any folders. Project system will include it. This is not needed

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

#fixed

@shahabhijeet
Copy link
Copy Markdown
Contributor

@solankisamir as long as Kedar or someone from your team approves, this is good to merge

@solankisamir
Copy link
Copy Markdown
Member Author

@shahabhijeet can you take a look?

@shahabhijeet
Copy link
Copy Markdown
Contributor

@azuresdkci test this please

@KedarJoshi
Copy link
Copy Markdown

#sign-off

@shahabhijeet shahabhijeet merged commit 7006bb6 into Azure:psSdkJson6 Aug 10, 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.

4 participants