Actions: Deploy preview docs #66

codeallthethingz · 2024-11-24T05:04:23Z

This PR creates a set of hidden docs on readme.com with a version matching the current version of the docs up to the minor version + the branch name made safe for metadata on a semver version.

For example, this branch: 0.0-deploy-preview-docs

While building it puts a sticky note on the PR's comments

Building:

Finished:

Failure:
Similar to building but with a ❌

This PR needs some action secrets / vars to be created.

README_API_KEY
IMAGE_PATH=thousandbrainsproject/tbp.monty/refs/heads/main/docs/figures
DOCS_URL_PREFIX=https://thousandbrainsproject.readme.io/

Note

Currently preview docs are not deleted when they are merged to main, this will be done in a separate PR.

codeallthethingz · 2024-11-24T14:38:54Z

@tristanls I'm flummoxed. None of the docs.yml actions are running and I need help to figure out why.

tristanls · 2024-11-24T15:12:23Z

@codeallthethingz actions will not pass secrets on pull_request trigger event. It'll need to be pull_request_target and in main before it passes secrets. This is what happened with CLA check.

codeallthethingz · 2024-11-24T15:15:05Z

Ah, right! Thanks.

codeallthethingz · 2024-11-24T15:26:49Z

I tried to add a trigger of both pull_request and pull_reqeust_target the docs.yml, but I feel like the entire file is skipped if there is a reference to a secret in it, which I think means I need to split the docs.yml into two - one for the secretless check-docs and one for the secretfull preview-docs. Do I have that right?

codeallthethingz · 2024-11-24T18:49:28Z

@tristanls I split the file into two, and the action that need a secret, triggers on pull_request_target. Let me know if this is the right approach overall.

If the above is correct:

There is now a repeating chunk of yaml should_run_docs that could be extracted into a common file.
Also, I wasn't sure of the naming convention now we have two "docs" type actions. Let me know what might be a good naming strategy.

Also, this can all wait till Monday.

codeallthethingz · 2024-11-24T18:46:01Z

.github/workflows/docs-check.yml

@@ -90,7 +91,7 @@ jobs:
            echo $changed_file
            if [[ $changed_file == docs/* ]] ||
               [[ $changed_file == tools/github_readme_sync/* ]] ||
-               [[ $changed_file == .github/workflows/docs.yml ]]; then
+               [[ $changed_file == .github/workflows/docs* ]]; then


This might be too greedy a match?

issue: The rationale for including the workflow file path here is to ensure the workflow is triggered when the workflow file changes, even if nothing else changes. I don't think we should trigger this workflow if some unrelated workflow file changes.

tristanls

Regarding naming suggestions, I think Docs and Docs Preview make sense.

Docs leads to check-docs, should-run-docs.
Docs Preview leads to deploy-docs-preview.

These appear coherent to me.

tristanls · 2024-11-25T20:26:00Z

.github/workflows/docs-preview.yml

+    steps:
+      - name: Create initial PR comment
+        if: ${{ needs.should_run_docs.outputs.should_run_docs == 'true' }}
+        uses: marocchino/sticky-pull-request-comment@v2


issue (blocking) Our security policy does not permit the execution of actions by unverified creators.

tristanls · 2024-11-25T20:28:08Z

.github/workflows/docs-check.yml

@@ -1,4 +1,4 @@
-name: Docs
+name: Docs Check


issue: Changing this calls for changing the job names per our naming convention.

I've referenced our naming convention in other PRs, but having it in the style guide is probably good. While it is not there yet, this PR adds it and can be a reference: #69.

tristanls · 2024-11-25T20:29:00Z

.github/workflows/docs-check.yml

@@ -18,6 +18,7 @@ jobs:
    runs-on: ubuntu-latest
    needs:
      - should_run_docs
+    if: github.event_name == 'pull_request'


question: Why would we not check the docs when merging to main?

tristanls · 2024-11-25T20:31:26Z

.github/workflows/docs-check.yml

@@ -90,7 +91,7 @@ jobs:
            echo $changed_file
            if [[ $changed_file == docs/* ]] ||
               [[ $changed_file == tools/github_readme_sync/* ]] ||
-               [[ $changed_file == .github/workflows/docs.yml ]]; then
+               [[ $changed_file == .github/workflows/docs* ]]; then


issue: The rationale for including the workflow file path here is to ensure the workflow is triggered when the workflow file changes, even if nothing else changes. I don't think we should trigger this workflow if some unrelated workflow file changes.

tristanls · 2024-11-25T20:31:57Z

.github/workflows/docs-preview.yml

+      - main
+
+jobs:
+  deploy_preview_docs:


issue: Should follow the naming convention described in #69.

tristanls · 2024-11-25T20:34:39Z

.github/workflows/docs-preview.yml

+        run: |
+          export PATH="$HOME/miniconda/bin:$PATH"
+          source activate tbp.monty
+          echo "VERSION=$(python -m tools.print_version.cli minor)" >> $GITHUB_ENV


suggestion (non-blocking): It may be safer to namespace when using a shared environment, e.g., MONTY_VERSION.

tristanls · 2024-11-25T20:35:05Z

.github/workflows/docs-preview.yml

+          # Remove trailing dash if present
+          echo "BRANCH_NAME=$(echo ${GITHUB_HEAD_REF} | tr -c '[:alnum:]' '-' | sed 's/-*$//')" >> $GITHUB_ENV
+          # Get current date in ISO format
+          echo "DATE=$(date -u +'%Y-%m-%d %H:%M UTC')" >> $GITHUB_ENV


suggestion (non-blocking): It may be safer to namespace when using a shared environment, e.g., MONTY_DATE.

tristanls · 2024-11-25T20:36:20Z

.github/workflows/docs-preview.yml

+
+            Check the [workflow logs](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) for details.
+
+  should_run_docs:


issue: Should follow the naming convention described in #69.

tristanls · 2024-11-25T20:43:22Z

.github/workflows/docs-preview.yml

+
+            Check the [workflow logs](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) for details.
+
+  should_run_docs:


suggestion: For non-required checks, we do not need to go through an artisanal setup of "should run," as seen in other workflows.

We can use the default paths filtering mechanism when a check is not required. See https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#onpushpull_requestpull_request_targetpathspaths-ignore.

The artisanal "should run" code is necessary elsewhere because when a workflow is skipped using the paths mechanism above, it registers as "skipped" instead of "success." However, a required check must "succeed" and cannot be "skipped."

codeallthethingz added 14 commits November 21, 2024 05:55

rename title to match slug

256de9a

Merge remote-tracking branch 'upstream/main'

0fead22

Merge remote-tracking branch 'upstream/main'

52c0aa3

Merge remote-tracking branch 'upstream/main'

d94d18d

Merge remote-tracking branch 'upstream/main'

d3c39ba

Merge remote-tracking branch 'upstream/main'

37ad1d4

action

f55b0fc

action

9f6db94

action

500a318

action

f7f90fb

action

3658610

action

8c7ede2

action

b53a36e

action

b2a05e9

codeallthethingz marked this pull request as draft November 24, 2024 14:11

remove blank line

dd94c47

codeallthethingz marked this pull request as ready for review November 24, 2024 14:27

codeallthethingz requested a review from tristanls November 24, 2024 14:27

codeallthethingz assigned tristanls Nov 24, 2024

action

bff7b03

codeallthethingz added infrastructure Changes to infrastructure triaged This issue or pull request was triaged labels Nov 24, 2024

codeallthethingz added 2 commits November 24, 2024 06:36

action

b87c637

action

a2da615

codeallthethingz added 2 commits November 24, 2024 07:19

check to run on PR preview to run on PRT

f4e2566

action

ce25a41

codeallthethingz added 6 commits November 24, 2024 10:30

action

a40f252

action

f4b5ed7

action

5830a84

action

193fc80

action

9d47904

action

249831d

trailing space

6962eaf

codeallthethingz commented Nov 24, 2024

View reviewed changes

tristanls reviewed Nov 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Actions: Deploy preview docs #66

Actions: Deploy preview docs #66

codeallthethingz commented Nov 24, 2024 •

edited

Loading

codeallthethingz commented Nov 24, 2024

tristanls commented Nov 24, 2024

codeallthethingz commented Nov 24, 2024

codeallthethingz commented Nov 24, 2024

codeallthethingz commented Nov 24, 2024

codeallthethingz Nov 24, 2024

tristanls Nov 25, 2024

tristanls left a comment

tristanls Nov 25, 2024

tristanls Nov 25, 2024

tristanls Nov 25, 2024

tristanls Nov 25, 2024

tristanls Nov 25, 2024

tristanls Nov 25, 2024

tristanls Nov 25, 2024

tristanls Nov 25, 2024

tristanls Nov 25, 2024


		Check the [workflow logs](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) for details.

		should_run_docs:

Actions: Deploy preview docs #66

Are you sure you want to change the base?

Actions: Deploy preview docs #66

Conversation

codeallthethingz commented Nov 24, 2024 • edited Loading

codeallthethingz commented Nov 24, 2024

tristanls commented Nov 24, 2024

codeallthethingz commented Nov 24, 2024

codeallthethingz commented Nov 24, 2024

codeallthethingz commented Nov 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tristanls left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeallthethingz commented Nov 24, 2024 •

edited

Loading