Create configlet-sync.yml #212
There are no files selected for viewing
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,87 @@ | ||||||
| name: Configlet Auto Sync | ||||||
|
|
||||||
| on: | ||||||
| workflow_call: | ||||||
|
|
||||||
| jobs: | ||||||
| sync-docs-metadata: | ||||||
| timeout-minutes: 10 | ||||||
| runs-on: ubuntu-24.04 | ||||||
|
|
||||||
| steps: | ||||||
| - name: Checkout repository | ||||||
| uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 | ||||||
|
|
||||||
| - name: Fetch configlet | ||||||
| run: ./bin/fetch-configlet | ||||||
|
|
||||||
| - name: Run configlet sync for files | ||||||
| run: ./bin/configlet sync --docs --metadata --filepaths -u -y | ||||||
|
|
||||||
| - name: Create pull request if changes | ||||||
| uses: peter-evans/create-pull-request@271a8d0340265f705b14b6d32b9829c1cb33d45e | ||||||
| with: | ||||||
| token: ${{ secrets.GITHUB_TOKEN }} | ||||||
| commit-message: "🤖 Auto-sync docs, metadata, and filepaths" | ||||||
| title: "🤖 Configlet sync: docs, metadata, and filepaths" | ||||||
| body: | | ||||||
| This PR was generated automatically by a scheduled workflow. | ||||||
|
|
||||||
| It includes updates from `configlet sync` for: | ||||||
| - 📄 Documentation | ||||||
| - 🧭 Metadata | ||||||
| - 🗂️ Filepaths | ||||||
|
|
||||||
| Please review and merge if everything looks good! | ||||||
| branch: configlet-auto-sync | ||||||
| delete-branch: true | ||||||
|
|
||||||
| check-test-sync: | ||||||
| timeout-minutes: 10 | ||||||
| runs-on: ubuntu-24.04 | ||||||
|
|
||||||
| steps: | ||||||
| - name: Checkout repository | ||||||
| uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 | ||||||
|
|
||||||
| - name: Fetch configlet | ||||||
| run: ./bin/fetch-configlet | ||||||
|
|
||||||
| - name: Run configlet sync for test and capture output | ||||||
| id: sync_test | ||||||
| shell: bash {0} | ||||||
| run: | | ||||||
| configlet_raw_output="$(./bin/configlet sync --tests)" | ||||||
| exit_code=$? | ||||||
| printf "exit_code=%d\n" "$exit_code" >> "$GITHUB_OUTPUT" | ||||||
| printf "output<<EOF\n%s\nEOF\n" "$configlet_raw_output" >> "$GITHUB_OUTPUT" | ||||||
| printf "configlet output:\n%s\n" "$configlet_raw_output" | ||||||
|
|
||||||
| - name: Format test sync output and find existing issue | ||||||
| id: find_issue | ||||||
| if: ${{ steps.sync_test.outputs.exit_code != 0 }} | ||||||
|
There was a problem hiding this comment. If you go with my suggestion, this would become:
Suggested change
|
||||||
| env: | ||||||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||||||
| run: | | ||||||
| printf '```\n%s\n```\n' "${{ steps.sync_test.outputs.output }}" > /tmp/sync-test-output.txt | ||||||
|
|
||||||
| ISSUE_TITLE="🚨 configlet sync --test found unsynced tests" | ||||||
| ISSUE_DATA=$(gh issue list --search "is:issue is:open in:title \"${ISSUE_TITLE}\" repo:${{ github.repository }}" --json number,title --jq '.[0]') | ||||||
|
|
||||||
| if [[ -z "$ISSUE_DATA" ]]; then | ||||||
| printf "No open issue found with title: '%s'. A new one will be created.\n" "${ISSUE_TITLE}" | ||||||
| printf "issue_number=\n" >> "$GITHUB_OUTPUT" | ||||||
| else | ||||||
| ISSUE_NUMBER=$(jq -r '.number' <<< "$ISSUE_DATA") | ||||||
| printf "Found existing issue number: %s for title: %s\n" "$ISSUE_NUMBER" "$ISSUE_TITLE" | ||||||
| printf "issue_number=%s\n" "$ISSUE_NUMBER" >> "$GITHUB_OUTPUT" | ||||||
| fi | ||||||
|
|
||||||
| - name: Create or Update issue if tests are not synced | ||||||
| if: ${{ steps.sync_test.outputs.exit_code != 0 }} | ||||||
|
There was a problem hiding this comment. Dito:
Suggested change
|
||||||
| uses: peter-evans/create-issue-from-file@e8ef132d6df98ed982188e460ebb3b5d4ef3a9cd | ||||||
| with: | ||||||
| token: ${{ secrets.GITHUB_TOKEN }} | ||||||
| title: "🚨 configlet sync --test found unsynced tests" | ||||||
| content-filepath: /tmp/sync-test-output.txt | ||||||
| issue-number: ${{ steps.find_issue.outputs.issue_number || '' }} | ||||||
|
There was a problem hiding this comment. Will |
||||||
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.
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.
There's no need to turn off
set -evia a customshellentry. Bash underset -ecan handle commands whose non-zero exit code is handled by the calling script and therefore not considered an unexpected failure that should lead to termination (which is whatset -eis for). Just use some conditional statement such asifto use this. I believe here it would even increase readability, because instead of storing the raw exit code, you could save a boolean with a better name and use that below.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.
This implies that the string "true" or "false" is better than 0 or 1 somehow. This is when m shell code. Shell exit codes are very well understood. This also adds more code and sets up additional outputs that may or may not be set. I'm not sure this approach is necessarily better or more readable.
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.
Alternatively, you can split the handling of configlet's exit code and writing to
$GITHUB_OUTPUT:This would add configlet's output to
$GITHUB_OUTPUTeven if it is not used, but for better readability would be acceptable (it's also what your current implementation does).If you find the
if ... then ... else ... fitoo verbose, I think something like this instead would also count as idiomatic Bash code (the exception aboutset -ein conditionals holds here as well):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.
Using $? is also perfectly readable and idiomatic bash 🙂
Uh oh!
There was an error while loading. Please reload this page.
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.
When used as a boolean in a GH actions workflow, it is better, because boolean exists as a datatype and anything else is coerced when used in a boolean expression (this is what the
if:s in the subsequent steps are), see here. Directly sayingtrueorfalseavoids this coercion.Sure, but besides success being zero and non-success something else, it depends on the tool. Without knowing configlet, from this snippet alone I would not know that a non-zero exit code means there are unsynced tests. I would have to read the rest of the workflow file to learn that. Using a more descriptive key than
exit_codedoes convey more information.More code is not necessarily worse if it's easy to read. Even somebody with little Bash experience would know what the
if ... then ... else ... fidoes. As an alternative, I suggested the option with the||shortcut that adds less code.Not true. I replaced one output (
exit_code) with another (unsynced_tests_found). The statement about maybe being set or not applies just as well toexit_code.I guess that depends on perspective. Ultimately it was just a suggestion.
My main point was that it is not necessarily to override GH's reasonable
set -edefault. That you can use more descriptive variable names in the process was an added benefit, IMHO at least.(Edit: Many typos fixed.)
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.
set -eas a reasonable default is also a matter of debate. See https://mywiki.wooledge.org/BashFAQ/105