Show individual doc differences for rendered documentation preview on PRs #2647
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
🧱 stack: documentation
openverse-bot
added
🟩 priority: low
|
Full-stack documentation: https://docs.openverse.org/_preview/2647 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. Changed files 🔄: |
AetherUnbound
force-pushed
the
feature/individual-doc-differences
branch
from
0b41ead to
d969c79
Compare
AetherUnbound
force-pushed
the
feature/individual-doc-differences
branch
3 times, most recently
from
47998bb to
f77076d
Compare
obulat
approved these changes
Jul 31, 2023
There was a problem hiding this comment.
Awesome work! A lot of tricky edge-cases, too :) This will review the friction from reviewing the docs changes.
I wonder if we could add a change to a storybook story here to see if the link is generated. I've added a patch that updates the story for VLicenseElements, removing the deprecated default parameters and replacing them with args.
| "CNAME", | ||
| "*.pickle", | ||
| "*.doctree", | ||
| "*.inv", |
There was a problem hiding this comment.
Suggested change
| "*.inv", | |
| "*.inv", | |
| "*.txt", | |
| "*.map", |
These two also need to be excluded based on the comment to this PR. I'm not so sure about the txt, though, would there ever be txt files added that we would like to review?
There was a problem hiding this comment.
Would it be easier to only include relevant files (is it just HTML?), instead of playing whack-a-mole with file types?
There was a problem hiding this comment.
@sarayourfriend I've tried a few different attempts based off this approach (example) but for some reason diff is still comparing files it should be excluding 😕 it's not ideal, but in the interest of not spending hours on this I'll add the additional exclusions and keep the current approach.
There was a problem hiding this comment.
I've also added some handling in 8e5ff36 to ensure we only display valid HTML pages in the diff.
dhruvkb
approved these changes
Aug 1, 2023
|
@dhruvkb to your point, there seem to be a few existing actions which allow for listing changed files, for example: https://github.com/marketplace/actions/changed-files @AetherUnbound's custom approach is still better, I reckon, since it does the link formatting we need. |
|
The action we currently use to determine where changes have happened in the repo can also provide the list of changed files: https://github.com/dorny/paths-filter#usage If we add the |
The former will apparently change all files if a new page is added, which is not ideal!
AetherUnbound
force-pushed
the
feature/individual-doc-differences
branch
from
2b515d2 to
ed74c93
Compare
|
Size Change: 0 B Total Size: 877 kB ℹ️ View Unchanged
|
AetherUnbound
force-pushed
the
feature/individual-doc-differences
branch
from
876b38e to
8e5ff36
Compare
|
Pushing one last commit to remove the example file, then merging this! |
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes
Fixes #2646 by @AetherUnbound
Description
This PR adds some new logic to the emit docs step to determine which files have been added or changed, and notes them in the docs preview. The easiest way I found to accomplish this was with the
diffCLI tool, but the output of that tool is not necessarily machine-readable and thus more complicated than a series of bash commands inside a GitHub Actions'runblock. I instead put all the logic inside a python file which is called by the workflow. This emits a comment (preserved for posterity) like the one below:I knowingly did not add tests for this script - we don't yet have tests (but want to add them as part of #1916), and setting up testing for the automation scripts would be its own bolus of work. I didn't want to include that in this PR, so we can tackle that as part of #1916 down the line.
Testing Instructions
Check the emit docs comment!
Checklist
Update index.md).main) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin