Fix URLs to C++ Core Guidelines #5605
Conversation
Replace repo-url with published site URL. The ids used are not valid on the GitHub repository (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md), but are valid on the published site (https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines).
|
@microsoft-github-policy-service agree |
PRMerger Results
|
There was a problem hiding this comment.
Some of the anchors work on GitHub, while others don't. However, there are a couple of links that point to the top of the page instead of the respective subsections. Those could be fixed in another commit or a follow-up PR.
Overall, good change to standardize the links to the published version, since the GitHub version was quite laggy due to the sheer size.
docs/code-quality/c26433.md
Outdated
| @@ -11,7 +11,7 @@ helpviewer_keywords: ["C26433"] | |||
|
|
|||
| ## C++ Core Guidelines | |||
|
|
|||
| [C.128: Virtual functions should specify exactly one of virtual, override, or final](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md) | |||
| [C.128: Virtual functions should specify exactly one of virtual, override, or final](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines) | |||
There was a problem hiding this comment.
I'll check if there are more without an explicit anchor and fix those.
|
Learn Build status updates of commit 52bb410: ✅ Validation status: passed
This comment lists only the first 25 files in the pull request. |
PRMerger Results
|
I now see that some links use an explicit anchor (defined in the .md), and others an implicit anchor (generated by the publishing tool, Jekyll). For example "C.128: Virtual functions should specify exactly one of virtual, override, or final" is both:
If you want, given I'm now touching all those files anyway, I could also make them all use the explicit anchor. |
|
Can you review the proposed changes? Important: When the changes are ready for publication, adding a #label:"aq-pr-triaged" |
prmerger-automator
bot
added
the
aq-pr-triaged
|
@v-dirichards OK, I'm only wondering about my last comment regarding the inconsistent use of explicit and implicit (Jekyll generated, unstable IDs). I actually have a change for that ready to replace all those with the explicit IDs, but maybe you prefer that as a separate follow-up PR? |
That question is way outside of my area of expertise. I'll defer to Tyler for that. |
|
@Mrottevell (Mark) - thank you for doing this work. Regarding your question - let's use explicit IDs. Go ahead and submit that work and we'll do this all in one go. I suppose there is a chance of being out-of-date by linking to the published site which can lag. But I think the benefits of going to the published site outweigh the negatives. Besides, when we add new links we can ensure that the published site is caught up before adding new links. This is good - thank you! |
Also replaced some explicit IDs with more specific explicit IDs for Bounds.N and Type.N (to the specific rule, instead of the section containing the rule).
|
Learn Build status updates of commit 7b5d19e: ✅ Validation status: passed
This comment lists only the first 25 files in the pull request. |
PRMerger Results
|
OK, I just pushed the commit. I also replaced some explicit IDs with more specific explicit IDs (for the Type.N and Bounds.N rules, for some rules it linked to the section containing the rules, and for others it linked to the specific rule, so I changed them to always link to the specific rule).
The workflow file of the C++ Core Guidelines repository publishes on pushes to master, so the only delays I expect would be in caching.
You're welcome :) |
|
Invalid command: '#sign-off'. Only the assigned author of one or more file in this PR can sign off. @TylerMSFT, @kylereedmsft, @Rastaban |
|
(I guess I misunderstood the comment from Diane regarding the sign-off comment :) ) |
|
Invalid command: '#sign-off'. Only the assigned author of one or more file in this PR can sign off. @TylerMSFT, @kylereedmsft, @Rastaban |
The sign-off comment is meant for the reviewer (in this repo pretty much only Tyler) to signal to the PR review team that this PR is ready to merge. |
|
#sign-off |
This PR replaces the URLs to the C++ Core Guidelines using the GitHub repository with the published site URL.
Some ids used in the links are not valid on the markdown file in the GitHub repository (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md), but they are valid on the published site (https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines).
This fix will make the URLs actually useful, as it will link you directly to the linked item, instead of the top of the file in the repository.