-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Updated endpoint paths after link normalization #7479
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
Conversation
Removed duplicate/trailing hash in URLs, duplicate parentheses and unnecessary backslashes
The regular expression `[|]{2}FIXME[|]{2}([^|]+)[|]{2}([^|]+)[|]{2}` should be used to replace the placeholders To get back to the previous state use `{$1$2}` as replacement pattern, but those were broken
Those links got skipped by the check links script
With commit fe1e87a I should have catched all broken endpoint paths. I replaced them with a placeholder, that could safely be replaced using the regular expression Previous markdown as reference (archived result)## Get Guild Audit Log % GET /guilds/{guild.id#DOCS_RESOURCES_GUILD/guild-object}/audit-logs Current markdown as reference (current result)## Get Guild Audit Log % GET /guilds/{guild.id/docs/resources/guild#guild-object}/audit-logs Current placeholder markdown within this pull request## Get Guild Audit Log % GET /guilds/||FIXME||guild.id||/docs/resources/guild#guild-object||/audit-logs Applying the replacement patternAssuming as an example the links would work like default markdown links, the appropriate replacement pattern would be The resulting markdown of the example line would be ## Get Guild Audit Log % GET /guilds/[{guild.id}](/docs/resources/guild#guild-object)/audit-logs |
In commit 65ed3d2 I took a look at the Help Center links and tried to standardize them by setting the locale to Also I removed the article name from the URL as this can change over time even though it was quite helpful to have the name of the article to search for an appropriate replacement in some cases. In my personal opinion the context in which a link occurs is clear enough most of the time that the article name can be left out and the link can be replaced easier if there are not multiple article names within the URLs. Some additional notesThe URL
Kind of the same thing applies to discord-api-docs/docs/change-log/2022-09-01-message-content-is-a-privileged-intent.md Line 17 in aa79300
Discord Developer Terms of Service:
Discord Developer Policy:
This commit can of course be reverted if these are unwanted changes. I just wanted to continue with the normalization/standardization of URLs within the developer documentation as I was already working my way trough all the URLs within the repository. I also want to point out that I slightly adapted the following two lines (replaced "workarounds" with "alternatives" as the article has been renamed this way too and "DDevs server" with "official Discord Developers Server" as this is how this reference is called in the most cases) discord-api-docs/docs/tutorials/upgrading-to-application-commands.md Lines 302 to 303 in fe1eeae
|
Please pause working on this one! A lot of the link updates you have are removing things we actually want :) On the issues with the route urls, there is an incoming fix that addresses that problem. I appreciate the callout! |
No worries, adjusting the links using the placeholder is zero time effort (creating them was 😌). Looking through the changes in pull request #7485, I noticed a few broken ones. I have just added the new format to compare the results. I'll undo the link changes now, and then it should be limited to what this pull request was originally intended for. |
The following changes are introduced by this pull request now:
I've reviewed all changes myself manually once again and think nothing of this should break anything. I am also able to remove things from this list in the code, just let me know or remove it yourself. |
I stumbled accross #7490 too during this pull request as it looked inconsistent. Wrapped those in inline code rather than escaping like it's done for other endpoint paths. Left out the method because it would be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of small change requests. In the future - it will be a lot easier to land these if they're smaller and single purpose!
docs/change-log/2018-01-23-deprecation-accept-invite-endpoint.md
Outdated
Show resolved
Hide resolved
This reverts commit fe1eeae.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
As #7475 did break endpoint paths that contained a link to another resource, I attempted to fix this. As I was working my way trough the git diff of commit a62c908 I noticed some other small things, I attempted to fix as well. Resolves #7478