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.

Sign up for GitHub

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

Tools: Preview was using main (still) #143

Merged
merged 15 commits into from
Jan 24, 2025
Merged

Tools: Preview was using main (still) #143

Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
d83db3c
fix: checkout the branch to preview
codeallthethingz Jan 17, 2025
53c919e
just run the tool on the docs
codeallthethingz Jan 17, 2025
159b759
action
codeallthethingz Jan 17, 2025
17ea165
action
codeallthethingz Jan 17, 2025
569756b
merge main
codeallthethingz Jan 17, 2025
ab0349e
Merge branch 'main' into bug-preview-does-not
codeallthethingz Jan 17, 2025
85ee084
revert
codeallthethingz Jan 17, 2025
11828f9
pull_request
codeallthethingz Jan 17, 2025
bcaee8f
action
codeallthethingz Jan 17, 2025
f9e3ad7
action
codeallthethingz Jan 17, 2025
4b562e6
action
codeallthethingz Jan 17, 2025
7ffc50c
revert to target
codeallthethingz Jan 17, 2025
a75c3d4
Merge branch 'main' into bug-preview-does-not
codeallthethingz Jan 20, 2025
76bcb04
Merge branch 'main' into bug-preview-does-not
tristanls Jan 21, 2025
1f5a071
Merge branch 'main' into bug-preview-does-not
codeallthethingz Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions .github/workflows/docs_preview.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,13 @@ jobs:
fetch-depth: 0
lfs: true
path: tbp.monty
ref: ${{ github.event.pull_request.merge_commit_sha }}

- name: Checkout PR branch
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }}
fetch-depth: 0
lfs: true
path: pr_docs
- name: Create initial PR comment
uses: ./tbp.monty/.github/actions/pin_comment
with:
Expand Down Expand Up @@ -73,16 +78,14 @@ jobs:
uses: ./tbp.monty/.github/actions/get_preview_info
with:
user_login: ${{ github.event.pull_request.user.login }}

- name: Deploy docs
working-directory: tbp.monty
run: |
export PATH="$HOME/miniconda/bin:$PATH"
source activate tbp.monty
export README_API_KEY=${{ secrets.README_API_KEY }}
export IMAGE_PATH=${{ vars.IMAGE_PATH }}
python -m tools.github_readme_sync.cli upload docs "${{ steps.preview_info.outputs.monty_version }}-${{ steps.preview_info.outputs.branch_name }}"

python -m tools.github_readme_sync.cli upload ../pr_docs/docs "${{ steps.preview_info.outputs.monty_version }}-${{ steps.preview_info.outputs.branch_name }}"
Copy link
Contributor

@tristanls tristanls Jan 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

question: I don't understand where ../pr_docs/docs comes from. Why does this work?

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's in the sparse checkout from above.

The flow is

  • check out the main branch using checkout action into ~/tbp.monty/
  • check out the PR's branch using the head reference into ~/pr_docs/ (was using sparse checkout but could be the entire project as you suggest)
  • run the tool from the main branch code, but upload the docs from the PR branch. ../pr_docs/docs

Thinking about it last night, this does put us at some risk of markdown/regex attacks as the PR could introduce some documentation that our regex parsing/markdown lib doesn't deal with very well and explodes. https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually, ignore that, the code has gone... Lemme find it.

Copy link
Contributor Author

@codeallthethingz codeallthethingz Jan 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

ok! That was embarrassing. The code is now back @tristanls

Copy link
Contributor

Choose a reason for hiding this comment

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

note: Now I see what's going on 🙂!

- name: Update PR comment on success
if: success()
uses: ./tbp.monty/.github/actions/pin_comment
Expand Down
Loading