Skip to content

Conversation

@travist13
Copy link
Contributor

The powershell generation module has been broken for compute for 5 months. These are the fixes to get that module ready for 2108

@travist13 travist13 requested a review from a team as a code owner August 12, 2021 17:35
@travist13 travist13 requested review from TheOnlyWei and removed request for a team August 12, 2021 17:35
@travist13 travist13 force-pushed the user/travist/fixpsmodule branch from b6dbcea to f5e556d Compare August 12, 2021 18:05
@travist13 travist13 force-pushed the user/travist/fixpsmodule branch from f5e556d to 592d7ca Compare August 12, 2021 18:36
Copy link
Member

@bganapa bganapa left a comment

Choose a reason for hiding this comment

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

Thanks for the PR and working through the issues to improve the docs.

I have added fewcomments

@bganapa bganapa requested a review from FireDefend August 17, 2021 19:05
Copy link
Contributor Author

@travist13 travist13 left a comment

Choose a reason for hiding this comment

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

Updated per PR comments. I left 3 unresolved comments but responded to them.

@bganapa
Copy link
Member

bganapa commented Aug 19, 2021

@travist13 there are conflicts as my PR is already merged in to dev. Can you pls resolve, for the Test json files, you could simply take your changes and ignore whatever is committed in dev branch as you have rerecorded them

@travist13
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 127 in repo Azure/azurestack-powershell

@TheOnlyWei
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@bganapa
Copy link
Member

bganapa commented Aug 19, 2021

@FireDefend COuld you pls review this PR and approve?

wmbt96
wmbt96 previously approved these changes Aug 20, 2021
Copy link

@wmbt96 wmbt96 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for removing the old features examples and updating the docs @travist13

@travist13
Copy link
Contributor Author

@FireDefend Could you please take a look at my PR and let me know if any changes are necessary? Thanks!

Copy link

@byue byue left a comment

Choose a reason for hiding this comment

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

src/Azs.Compute.Admin/docs/Update-AzsComputeGlobalFeatureSetting.md (line 87)

Add documentation on meaning of 3 string values:

Possible string values of GlobalFeatureSetting are Enabled, Disabled, and TenantSubscriptionLevel. Enabled/Disabled will override features that are enabled with tenant subscription ID. TenantSubscriptionLevel will defer feature enablement to per tenant subscription ID enablement.

@travist13 travist13 dismissed stale reviews from wmbt96 via d3a2dfa August 20, 2021 18:04
@travist13
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 127 in repo Azure/azurestack-powershell

@bganapa
Copy link
Member

bganapa commented Aug 20, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@bganapa
Copy link
Member

bganapa commented Aug 23, 2021

/azp run

@bganapa bganapa enabled auto-merge (squash) August 23, 2021 20:25
@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@bganapa bganapa merged commit ad20aef into Azure:dev Aug 23, 2021
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