git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] ci: make the whitespace checker more robust
@ 2020-11-03 15:55 Johannes Schindelin via GitGitGadget
  2020-11-03 22:02 ` Junio C Hamano
  0 siblings, 1 reply; 2+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2020-11-03 15:55 UTC (permalink / raw)
  To: git; +Cc: Chris. Webster, Johannes Schindelin, Johannes Schindelin

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    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(-)

diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml
index 9d070b9cdf..80a961716b 100644
--- a/.github/workflows/check-whitespace.yml
+++ b/.github/workflows/check-whitespace.yml
@@ -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
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] ci: make the whitespace checker more robust
  2020-11-03 15:55 [PATCH] ci: make the whitespace checker more robust Johannes Schindelin via GitGitGadget
@ 2020-11-03 22:02 ` Junio C Hamano
  0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2020-11-03 22:02 UTC (permalink / raw)
  To: Johannes Schindelin via GitGitGadget
  Cc: git, Chris. Webster, Johannes Schindelin

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
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.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-11-03 22:03 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-03 15:55 [PATCH] ci: make the whitespace checker more robust Johannes Schindelin via GitGitGadget
2020-11-03 22:02 ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).