Skip to content
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.

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

Update Deploy-only-Service-Health-Alerts.md #171

Merged
merged 2 commits into from
May 13, 2024

Conversation

tagolovina
Copy link
Contributor

Edited Custom deployment step 5, change command and added note for Azure Cloud Shell

Edited Custom deployment step 5, change command and added note for Azure Cloud Shell
Copy link
Collaborator

@Brunoga-MS Brunoga-MS left a comment

Choose a reason for hiding this comment

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

@tagolovina : is the text in the note a command sample? if so, I would suggest to not make it a note, but clearly call it example

@arjenhuitema arjenhuitema added the documentation Improvements or additions to documentation label Mar 27, 2024
Added the following changes:
Include a note with the correct command for cloud shell
add a note that it doesnt work in cloud shell. 
Alternatively, for step 5 the following command can be used in cloud shell: 
az deployment mg create --name "amba-ServiceHealthOnly" --template-file ./patterns/alz/policyDefinitions/policies-sh.json --location $location --management-group-id $pseudoRootManagementGroup --parameters topLevelManagementGroupPrefix=contoso
Alternatively, for step 6 we can add a note indicating that its possible to create a parameter file instead of using a json-string.
@tagolovina
Copy link
Contributor Author

@Brunoga-MS , changed the note.

Copy link
Collaborator

@Brunoga-MS Brunoga-MS left a comment

Choose a reason for hiding this comment

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

LGTM now

Copy link
Contributor

@arjenhuitema arjenhuitema left a comment

Choose a reason for hiding this comment

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

LGTM

@arjenhuitema arjenhuitema merged commit acf50b7 into Azure:main May 13, 2024
2 of 4 checks passed
@tagolovina tagolovina deleted the patch-2 branch August 23, 2024 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants