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

Bryanzab/us28627 #68

Closed
wants to merge 9 commits into from
Closed

Bryanzab/us28627 #68

wants to merge 9 commits into from

Conversation

bzabber
Copy link
Contributor

@bzabber bzabber commented Nov 8, 2023

Overview/Summary

Replace this with a brief description of what this Pull Request fixes, changes, etc.

This PR fixes/adds/changes/removes

  1. Updates policy-initiavies.md to include ALB to Connectivity initiative sub section.
  2. Updates policy-initiative.md to include AGW to Landing Zone initiative sub section.

Breaking Changes

None

As part of this Pull Request I have

  • Read the Contribution Guide and ensured this PR is compliant with the guide
  • Checked for duplicate Pull Requests
  • Associated it with relevant GitHub Issues or ADO Work Items (Internal Only)
  • Ensured my code/branch is up-to-date with the latest changes in the main branch
  • Ensured PR tests are passing
  • Updated relevant and associated documentation (e.g. Contribution Guide, Docs etc.)

| Deploy_activitylog_VPNGateway_Delete | [deploy-activitylog-VPNGate-Del.json](../../../services/Network/vpnGateways/Deploy-ActivityLog-VPNG-Del.json) | deployIfNotExists |
| **Policy Name**| **Path to policy json file**| **Policy default effect** |
|-----------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------------------|
| Deploy_ERCIR_QosDropBitsInPerSecond_Alert | [deploy-ercir_qosdropsbitsin_alert.json](../../../../services/Network/expressRouteCircuits/Deploy-ERCIR-QOSDropsBitsIn-Alert.json) | deployIfNotExists |
Copy link
Contributor

Choose a reason for hiding this comment

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

The links are working now, so i´m wondering why the extra "../" was added

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was the only way for me to verify the links in my local copy of the repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a meeting with @JoeyBarnes this week to review the markdown linting but also review whether the additional ../ is required when testing locally. I can't get a local copy of hugo locally for some reason to check whether the links work within hugo.

@arjenhuitema arjenhuitema added the documentation Improvements or additions to documentation label Nov 10, 2023
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.

Hi Bryan, I reviewed the changes and I just have a question, I see that all links have been modified, however the links are currently working, can you review if this change is required.

@bzabber
Copy link
Contributor Author

bzabber commented Dec 9, 2023

@JoeyBarnes I've gone through the markdown files that have issues with the URLs and they URLs all work, (there were a few that had issues but I've since fixed them. I also tested the links when running the Hugo locally and they work then too.

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.

2 participants