Skip to contentSkip to conversation

ci: make the whitespace checker more robust

PATCH In review Johannes Schindelin via GitGitGadget 2 participants 2 messages Nov 3, 2020

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ci: make the whitespace checker more robust

From: Johannes Schindelin <[email protected]>

In 32c83afc2c69 (ci: github action - add check for whitespace errors, 2020-09-22), we introduced a GitHub workflow that automatically checks Pull Requests for whitespace problems.

However, when affected lines contain one or more double quote characters, this workflow failed to attach the informative comment because the Javascript snippet incorrectly interpreted these quotes instead of using the git log output as-is.

Let's fix that.

While at it, let's await the result of the createComment() function.

Finally, we enclose the log in the comment with ```...``` to avoid having the diff marker be misinterpreted as an enumeration bullet.

---
ci: make the whitespace checker more robust

I noticed that the checker failed to add a comment
[https://github.com/gitgitgadget/git/runs/1344883313?check_suite_focus=true]
in one of my PRs. Turns out that the double-quote characters in the log output made it fail.

One thing we discussed earlier whether the log should be pasted as pre-formatted text or not, and we fell on the side of not pre-formatting it. However, in my tests, this does not look right
[https://github.com/dscho/git/pull/18#issuecomment-721160985], and it looks much better pre-formatted
[https://github.com/dscho/git/pull/18#issuecomment-721167209] (even if we unfortunately lose the direct link to the commit
[https://github.com/dscho/git/commit/68317764849af81b17c4b31906da20bdf2c52082]
).

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-778%2Fdscho%2Ffix-whitespace-github-workflow-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-778/dscho/fix-whitespace-github-workflow-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/778

 .github/workflows/check-whitespace.yml | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
.github/workflows/check-whitespace.yml +4 -2
index 9d070b9cdf..80a961716b 100644
@@ -58,12 +58,14 @@ jobs:
     - name: Add Check Output as Comment
       uses: actions/github-script@v3
       id: add-comment
+      env:
+        log: ${{ steps.check_out.outputs.checkout }}
       with:
         script: |
-            github.issues.createComment({
+            await github.issues.createComment({
               issue_number: context.issue.number,
               owner: context.repo.owner,
               repo: context.repo.repo,
-              body: "Whitespace errors found in workflow ${{ github.workflow }}:\n\n${{ steps.check_out.outputs.checkout }}"
+              body: `Whitespace errors found in workflow ${{ github.workflow }}:\n\n\`\`\`\n${process.env.log.replace(/\\n/g, "\n")}\n\`\`\``
             })
       if: ${{ failure() }}

base-commit: 32c83afc2c69aa51b82aa223f2099389f1f0be0a

Signature · 1 line
gitgitgadget
#1 lore raw

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 9 lines · writes:

One thing we discussed earlier whether the log should be pasted as pre-formatted text or not, and we fell on the side of not pre-formatting it. However, in my tests, this does not look right
[https://github.com/dscho/git/pull/18#issuecomment-721160985], and it looks much better pre-formatted
[https://github.com/dscho/git/pull/18#issuecomment-721167209] (even if we unfortunately lose the direct link to the commit
[https://github.com/dscho/git/commit/68317764849af81b17c4b31906da20bdf2c52082]
).

What is shown in the log are lines from the source files that were checked, and we expect our source files are shown and edited in monospace with tabwidth=8, I think it does make sense to force the "pre-formatted" output.

Thanks.