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

Updated markdowns to correspond with the Bicep files #331

Merged
merged 23 commits into from
Oct 26, 2022

Conversation

johnlokerse
Copy link
Contributor

@johnlokerse johnlokerse commented Oct 8, 2022

This PR fixes/adds/changes/removes

  1. Update the markdown files to correspond with the parameters in the Bicep files.

As part of this Pull Request I have

@ghost ghost added the Needs: Triage 🔍 Needs triaging by the team label Oct 8, 2022
@johnlokerse johnlokerse changed the title Updated markdowns to correspond with the Biceps Updated markdowns to correspond with the Bicep files Oct 8, 2022
@jtracey93 jtracey93 added documentation and removed Needs: Triage 🔍 Needs triaging by the team labels Oct 8, 2022
@jtracey93
Copy link
Collaborator

@johnlokerse let us know when this is good to review. I've placed the do not merge label on it for now.

@johnlokerse
Copy link
Contributor Author

@jtracey93 PR is ready for review. I went through every markdown file to review if the information is correct according to what is stated in the Bicep files. Also identified breaking changes that I have made, these are commented by me in the PR.

Copy link
Collaborator

@jtracey93 jtracey93 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 this @johnlokerse just one change required I think as per the comments unresolved.

@jtracey93
Copy link
Collaborator

/azp run validateazcloud

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jtracey93
Copy link
Collaborator

@johnlokerse we got an error also on the validation pipeline but we know the name can be longer for these resources. Can we remove any places we have added min or max length as part of this PR please 👍

ERROR: {"code": "InvalidTemplate", "message": "Deployment template validation failed: 'The provided value for the template parameter 'parVirtualWanHubResourceId' is not valid. Length of the value should be less than or equal to '50'. Please see https://aka.ms/arm-template/#parameters for usage details.'.", "additionalInfo": [{"type": "TemplateViolation", "info": {"lineNumber": 14, "linePosition": 21, "path": "properties.template.parameters.parVirtualWanHubResourceId.maxLength"}}]}

@ghost ghost removed the Needs: Author Feedback label Oct 22, 2022
@johnlokerse johnlokerse requested a review from jtracey93 October 22, 2022 16:09
@johnlokerse
Copy link
Contributor Author

@johnlokerse we got an error also on the validation pipeline but we know the name can be longer for these resources. Can we remove any places we have added min or max length as part of this PR please 👍

ERROR: {"code": "InvalidTemplate", "message": "Deployment template validation failed: 'The provided value for the template parameter 'parVirtualWanHubResourceId' is not valid. Length of the value should be less than or equal to '50'. Please see https://aka.ms/arm-template/#parameters for usage details.'.", "additionalInfo": [{"type": "TemplateViolation", "info": {"lineNumber": 14, "linePosition": 21, "path": "properties.template.parameters.parVirtualWanHubResourceId.maxLength"}}]}

@jtracey93 Reverted my changes regarding minLength and maxLength

@jtracey93
Copy link
Collaborator

Hey @johnlokerse,

Thanks for this we are reviewing :)

@jtracey93
Copy link
Collaborator

/azp run validateazcloud

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Collaborator

@jtracey93 jtracey93 left a comment

Choose a reason for hiding this comment

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

LGTM thanks @johnlokerse

@jtracey93 jtracey93 linked an issue Oct 26, 2022 that may be closed by this pull request
@jtracey93 jtracey93 self-assigned this Oct 26, 2022
@jtracey93 jtracey93 merged commit e3af36b into Azure:main Oct 26, 2022
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.

🪲 Bug Report - Typo in a few docs
2 participants