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

WEB-2754 - format link fixes #15544

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

davidejones
Copy link
Member

@davidejones davidejones commented Oct 12, 2022

What does this PR do?

site-region can also have its own link scope.
As this wasn't being accounted for it would leak into the "main" section causing the format link to say there are duplicates

EDIT:
This PR:

  • removes the precommit hook usage of the format link script
  • removes the integration processing usage of the format link script
  • Keeps the reworked format link script for debugging purposes

Motivation

https://datadoghq.atlassian.net/browse/WEB-2754

Preview

Testing
Locally i tested the following file

local/bin/py/build/actions/format_link.py -f content/en/continuous_integration/setup_pipelines/custom_tags_and_metrics.md

Trying on all files with something like

 for f in $(find ./content/en -type f -iname '*.md'); do local/bin/py/build/actions/format_link.py -f "$f"; done

However we should test multiple file changes and see how the content changes.

EDIT:
We should now just test that integrations are displaying as intended on the preview:

https://docs-staging.datadoghq.com/david.jones/format-link-fixes/integrations/

Additional Notes


Reviewer checklist

  • Review the changed files.
  • Review the URLs listed in the Preview section.
  • Check images for PII
  • Review any mentions of "Contact Datadog support" for internal support documentation.

@github-actions github-actions bot added the Architecture Everything related to the Doc backend label Oct 12, 2022
@davidejones davidejones force-pushed the david.jones/format-link-fixes branch from f192cbf to 0756b5c Compare November 2, 2022 13:02
@davidejones davidejones force-pushed the david.jones/format-link-fixes branch 3 times, most recently from 49ad7b1 to ec0620e Compare January 9, 2023 12:48
@davidejones davidejones force-pushed the david.jones/format-link-fixes branch from 46e7cf5 to 31b5b50 Compare January 24, 2023 11:50
@hestonhoffman
Copy link
Contributor

Hi David, I ran this on a test file and tried to break it and it looks like it's working really well!

@hestonhoffman
Copy link
Contributor

hestonhoffman commented Jan 31, 2023

@davidejones I just tried this on a file I've been writing, and while it doesn't throw an error like the old script did (I think it's cos I'm using single line code-blocks), it doesn't renumber the references.

Here's the file for reference
high_memory_usage.md

@davidejones
Copy link
Member Author

@hestonhoffman So generally i've tried to make the script avoid re-ordering where possible for 2 reasons.

  • The diffs are easier to read on modified files if there are less changes
  • The order Isn't particularly important for being able to render the page

I can change this though, would docs prefer the same behavior as before?

@hestonhoffman
Copy link
Contributor

@davidejones Yeah, that makes sense. I prefer this behavior. It should prevent the issue where merging master into a feature bunch pulls a bunch of unrelated changes into the PR

@davidejones davidejones force-pushed the david.jones/format-link-fixes branch 2 times, most recently from e56fd16 to 36b3b42 Compare March 17, 2023 10:24
@davidejones davidejones force-pushed the david.jones/format-link-fixes branch from bb5854c to e685757 Compare April 5, 2023 17:25
@davidejones davidejones force-pushed the david.jones/format-link-fixes branch 5 times, most recently from 8f8ee08 to 114ed30 Compare April 17, 2023 10:45
@davidejones davidejones marked this pull request as ready for review April 17, 2023 11:07
@davidejones davidejones requested review from a team as code owners April 17, 2023 11:07
@davidejones davidejones added the Do Not Merge Just do not merge this PR :) label Apr 17, 2023
@@ -1,6 +1,6 @@
---
- config:
cache_enabled: true
cache_enabled: false
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be changed back for preview? @davidejones

@davidejones davidejones force-pushed the david.jones/format-link-fixes branch from 114ed30 to 003262d Compare April 24, 2023 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture Everything related to the Doc backend Do Not Merge Just do not merge this PR :) salmon_curry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants