Skip to content

Eventhub: Added parameters in New and Set Namespace cmdlet for BYOK support #12130

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
wyunchi-ms merged 15 commits into from
Jun 16, 2020
Merged

Eventhub: Added parameters in New and Set Namespace cmdlet for BYOK support #12130

wyunchi-ms merged 15 commits into from
Jun 16, 2020

Conversation

@v-Ajnava
Copy link

@v-Ajnava v-Ajnava commented Jun 12, 2020 •
edited
Loading

Description

Added optional parameter in New-AzEventHubNamespace and Set-AzEventHubNamespace for BYOK support

https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/628

Checklist

  • I have read the Submitting Changes section of CONTRIBUTING.md
  • The title of the PR is clear and informative
  • The appropriate ChangeLog.md file(s) has been updated:
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
  • The PR does not introduce breaking changes
  • If applicable, the changes made in the PR have proper test coverage
  • For public API changes to cmdlets:
    • a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
    • the markdown help files have been regenerated using the commands listed here

@v-Ajnava v-Ajnava requested a review from wyunchi-ms June 12, 2020 06:20
wyunchi-ms
wyunchi-ms reviewed Jun 12, 2020
View reviewed changes
src/EventHub/EventHub.sln
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "TestFx", "..\..\tools\TestFx\TestFx.csproj", "{BC80A1D0-FFA4-43D9-AA74-799F5CB54B58}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "KeyVault", "..\KeyVault\KeyVault\KeyVault.csproj", "{7A4B0770-E6C4-479F-9EF4-6C549F217B3C}"
Copy link
Contributor

@wyunchi-ms wyunchi-ms Jun 12, 2020

Choose a reason for hiding this comment

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

Please don't reference other module in module project. In most situation reference them in test project is enough.

Copy link
Author

@v-Ajnava v-Ajnava Jun 15, 2020

Choose a reason for hiding this comment

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

the reference is EventHub.sln, similar to how its done on Compute

src/EventHub/EventHub/help/New-AzEventHubNamespace.md Outdated

## SYNOPSIS
Creates an Event Hubs namespace.
{{ Fill in the Synopsis }}
Copy link
Contributor

@wyunchi-ms wyunchi-ms Jun 12, 2020

Choose a reason for hiding this comment

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

Please regenerate the help document. online version and SYNOPSIS should not be empty.

Copy link
Author

@v-Ajnava v-Ajnava Jun 15, 2020

Choose a reason for hiding this comment

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

updated

src/EventHub/EventHub/help/New-AzEventHubNamespace.md Outdated
external help file: Microsoft.Azure.PowerShell.Cmdlets.EventHub.dll-Help.xml
Module Name: Az.EventHub
online version: https://docs.microsoft.com/en-us/powershell/module/az.eventhub/new-azeventhubnamespace
online version:
Copy link
Contributor

@wyunchi-ms wyunchi-ms Jun 12, 2020

Choose a reason for hiding this comment

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

Please regenerate the help document. online version and SYNOPSIS should not be empty.

Copy link
Author

@v-Ajnava v-Ajnava Jun 15, 2020

Choose a reason for hiding this comment

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

updated

src/EventHub/EventHub/help/Set-AzEventHubNamespace.md Outdated
external help file: Microsoft.Azure.PowerShell.Cmdlets.EventHub.dll-Help.xml
Module Name: Az.EventHub
online version: https://docs.microsoft.com/en-us/powershell/module/az.eventhub/set-azeventhubnamespace
online version:
Copy link
Contributor

@wyunchi-ms wyunchi-ms Jun 12, 2020

Choose a reason for hiding this comment

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

Please regenerate the help document. online version and SYNOPSIS should not be empty.

Copy link
Author

@v-Ajnava v-Ajnava Jun 15, 2020

Choose a reason for hiding this comment

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

updated

src/EventHub/EventHub/help/Set-AzEventHubNamespace.md Outdated

## SYNOPSIS
Updates the specified Event Hubs namespace.
{{ Fill in the Synopsis }}
Copy link
Contributor

@wyunchi-ms wyunchi-ms Jun 12, 2020

Choose a reason for hiding this comment

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

Please regenerate the help document. online version and SYNOPSIS should not be empty.

Copy link
Author

@v-Ajnava v-Ajnava Jun 15, 2020

Choose a reason for hiding this comment

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

updated

@v-Ajnava
Copy link
Author

v-Ajnava commented Jun 15, 2020

EventGrid test refers to 'Microsoft.Azure.Management.EventHub' and EventGrid test were failing, so updated with latest 'Microsoft.Azure.Management.EventHub' in EventGrid Test project

wyunchi-ms
wyunchi-ms reviewed Jun 16, 2020
View reviewed changes
src/EventHub/EventHub/Cmdlets/Namespace/SetAzureEventHubNamespace.cs Outdated
/// </summary>
[Parameter(Mandatory = false, ParameterSetName = AutoInflateParameterSet, HelpMessage = "List of Key Properties, @(@(KeyName,KeyVaultUri,Keyversion),@(KeyName,KeyVaultUri,Keyversion))")]
[Parameter(Mandatory = false, ParameterSetName = NamespaceParameterSet, HelpMessage = "List of Key Properties, @(@(KeyName,KeyVaultUri,Keyversion),@(KeyName,KeyVaultUri,Keyversion))")]
public List<string []> KeyProperties { get; set; }
Copy link
Contributor

@wyunchi-ms wyunchi-ms Jun 16, 2020

Choose a reason for hiding this comment

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

It's nessary to follow the naming guideline of powershell. You can see some suggestion from here.

Copy link
Author

@v-Ajnava v-Ajnava Jun 16, 2020

Choose a reason for hiding this comment

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

changed to KeyProperty

@wyunchi-ms wyunchi-ms merged commit d6e9537 into Azure:master Jun 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wyunchi-ms wyunchi-ms wyunchi-ms approved these changes

Assignees

@v-Ajnava v-Ajnava

@wyunchi-ms wyunchi-ms

Labels

needs-review needs-revision

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@v-Ajnava @wyunchi-ms @azuresdkci