threads / patch / 58965

patch, 2 partsMake check-whitespace failures more helpful

Subject: [PATCH 0/2] Make check-whitespace failures more helpful

## tl;dr

18 messages between Dec 16, 2022 and Dec 21, 2022. Diffs are folded; open one to read it.

replies: 17people: 4as markdown or json

Chris. Webster via GitGitGadget· Dec 16, 2022, 08:31 UTC · lore

Add the errors to the job summary along with suggested commands to fix the problem. The commits and filenames are links.

This is for issue #1395. Sample job output [https://github.com/webstech/check-whitespace/actions/runs/3707382446]:

❌ A whitespace issue was found in one or more of the commits.
Run these commands to correct the problem:
1. git rebase --whitespace=fix aaa04a9
2. git push --force
Errors:
1. --- 5cd37f6 Remove annotations
   trailing.txt:4: trailing whitespace.
   +
   trailing.txt:2: new blank line at EOF.
Chris. Webster (2):
  Make `check-whitespace` failures more helpful
  Improve check-whitespace output
 .github/workflows/check-whitespace.yml | 57 +++++++++++++++++++++-----
 1 file changed, 46 insertions(+), 11 deletions(-)
base-commit: 57e2c6ebbe7108b35ba30184dcbcb6c34c929ad8
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1444%2Fwebstech%2Fwhitespace-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1444/webstech/whitespace-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1444
-- 
gitgitgadget
Chris. Webster via GitGitGadget· Dec 16, 2022, 08:32 UTC · re: Chris. Webster via GitGitGadget · lore

[PATCH 2/2] Improve check-whitespace output

From: "Chris. Webster" <chris@webstech.net>
A message in the step log will refer to the Summary output.
The job summary output now has links to the commits and files.
Signed-off-by: Chris. Webster <chris@webstech.net>
---
 .github/workflows/check-whitespace.yml | 34 +++++++++++++++++++-------
 1 file changed, 25 insertions(+), 9 deletions(-)
Show changes to .github/workflows/check-whitespace.yml +25 −9
diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml
index 3a99073bc33..da557fd5914 100644
--- a/.github/workflows/check-whitespace.yml
+++ b/.github/workflows/check-whitespace.yml
@@ -20,46 +20,62 @@ jobs:
     - name: git log --check
       id: check_out
       run: |
+        baseSha=${{github.event.pull_request.base.sha}}
         problems=()
         commit=
         commitText=
-        lastcommit=
+        commitTextmd=
+        goodparent=
         while read dash sha etc
         do
           case "${dash}" in
           "---")
             if test -z "${commit}"
             then
-              lastcommit=${sha}
+              goodparent=${sha}
             fi
             commit="${sha}"
             commitText="${sha} ${etc}"
+            commitTextmd="[${sha}](https://github.com/${{ github.repository }}/commit/${sha}) ${etc}"
             ;;
           "")
             ;;
           *)
             if test -n "${commit}"
             then
-              problems+=("" "--- ${commitText}")
+              problems+=("1) --- ${commitTextmd}")
               echo ""
               echo "--- ${commitText}"
               commit=
             fi
-            problems+=("${dash} ${sha} ${etc}")
-            echo "${problems[-1]}"
+            case "${dash}" in
+            *:[1-9]*:) # contains file and line number information
+              dashend=${dash#*:}
+              problems+=("[${dash}](https://github.com/${{ github.repository }}/blob/${{github.event.pull_request.head.ref}}/${dash%%:*}#L${dashend%:}) ${sha} ${etc}")
+              ;;
+            *)
+              problems+=("\`${dash} ${sha} ${etc}\`")
+              ;;
+            esac
+            echo "${dash} ${sha} ${etc}"
             ;;
           esac
-        done <<< $(git log --check --pretty=format:"---% h% s" ${{github.event.pull_request.base.sha}}..)
+        done <<< $(git log --check --pretty=format:"---% h% s" ${baseSha}..)
 
         if test ${#problems[*]} -gt 0
         then
           if test -z "${commit}"
           then
-            lastcommit=${{github.event.pull_request.base.sha}}
+            goodparent=${baseSha: 0:7}
           fi
-          echo "A whitespace issue was found in one or more of the commits." >$GITHUB_STEP_SUMMARY
+          echo "🛑 Please review the Summary output for further information."
+          echo "### :x: A whitespace issue was found in one or more of the commits." >$GITHUB_STEP_SUMMARY
           echo "" >>$GITHUB_STEP_SUMMARY
-          echo "Run \`git rebase --whitespace=fix ${lastcommit}\` and \`git push --force\` to correct the problem." >>$GITHUB_STEP_SUMMARY
+          echo "Run these commands to correct the problem:" >>$GITHUB_STEP_SUMMARY
+          echo "1. \`git rebase --whitespace=fix ${goodparent}\`" >>$GITHUB_STEP_SUMMARY
+          echo "1. \`git push --force\`" >>$GITHUB_STEP_SUMMARY
+          echo " " >>$GITHUB_STEP_SUMMARY
+          echo "Errors:" >>$GITHUB_STEP_SUMMARY
           for i in "${problems[@]}"
           do
             echo "${i}" >>$GITHUB_STEP_SUMMARY

-- 
gitgitgadget
Junio C Hamano· Dec 16, 2022, 10:13 UTC · re: Chris. Webster via GitGitGadget · lore

Re: [PATCH 2/2] Improve check-whitespace output

"Chris. Webster via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: "Chris. Webster" <chris@webstech.net>
> Subject: Re: [PATCH 2/2] Improve check-whitespace output

The same comment about specificity of the improvements applies to this one, too. Also, I forgot to point out that our usual commit title takes the form of "<area>: <description>", e.g.

	Subject: [PATCH 2/2] ci: show $X in check.whitespace output

where "show $X" is meant to be a more concrete phrase than "improve" what the change is about and how it improves the output.

> +          echo "Run these commands to correct the problem:" >>$GITHUB_STEP_SUMMARY
> +          echo "1. \`git rebase --whitespace=fix ${goodparent}\`" >>$GITHUB_STEP_SUMMARY
> +          echo "1. \`git push --force\`" >>$GITHUB_STEP_SUMMARY

It's a bit curious to see two "1." and not "1." followed by "2." here. Is this meant to be processed by markdown or something so we do not have to do the numbering ourselves, or something?

Show 5 quoted lines
> +          echo " " >>$GITHUB_STEP_SUMMARY
> +          echo "Errors:" >>$GITHUB_STEP_SUMMARY
>            for i in "${problems[@]}"
>            do
>              echo "${i}" >>$GITHUB_STEP_SUMMARY
Thanks.
Chris Webster· Dec 20, 2022, 00:33 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] Improve check-whitespace output

On Fri, Dec 16, 2022 at 2:13 AM Junio C Hamano <gitster@pobox.com> wrote:
> It's a bit curious to see two "1." and not "1." followed by "2."
> here.  Is this meant to be processed by markdown or something so we
> do not have to do the numbering ourselves, or something?
Yes, GITHUB_STEP_SUMMARY is treated as markdown.

Thanks for the feedback, ...chris.

Chris. Webster via GitGitGadget· Dec 16, 2022, 08:31 UTC · re: Chris. Webster via GitGitGadget · lore

[PATCH 1/2] Make `check-whitespace` failures more helpful

From: "Chris. Webster" <chris@webstech.net>

Add the errors to the job summary along with suggested commands to fix the problem.

Signed-off-by: Chris. Webster <chris@webstech.net>
---
 .github/workflows/check-whitespace.yml | 39 +++++++++++++++++++-------
 1 file changed, 29 insertions(+), 10 deletions(-)
Show changes to .github/workflows/check-whitespace.yml +29 −10
diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml
index ad3466ad16e..3a99073bc33 100644
--- a/.github/workflows/check-whitespace.yml
+++ b/.github/workflows/check-whitespace.yml
@@ -13,38 +13,57 @@ jobs:
   check-whitespace:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v3
       with:
         fetch-depth: 0
 
     - name: git log --check
       id: check_out
       run: |
-        log=
+        problems=()
         commit=
-        while read dash etc
+        commitText=
+        lastcommit=
+        while read dash sha etc
         do
           case "${dash}" in
           "---")
-            commit="${etc}"
+            if test -z "${commit}"
+            then
+              lastcommit=${sha}
+            fi
+            commit="${sha}"
+            commitText="${sha} ${etc}"
             ;;
           "")
             ;;
           *)
             if test -n "${commit}"
             then
-              log="${log}\n${commit}"
+              problems+=("" "--- ${commitText}")
               echo ""
-              echo "--- ${commit}"
+              echo "--- ${commitText}"
+              commit=
             fi
-            commit=
-            log="${log}\n${dash} ${etc}"
-            echo "${dash} ${etc}"
+            problems+=("${dash} ${sha} ${etc}")
+            echo "${problems[-1]}"
             ;;
           esac
         done <<< $(git log --check --pretty=format:"---% h% s" ${{github.event.pull_request.base.sha}}..)
 
-        if test -n "${log}"
+        if test ${#problems[*]} -gt 0
         then
+          if test -z "${commit}"
+          then
+            lastcommit=${{github.event.pull_request.base.sha}}
+          fi
+          echo "A whitespace issue was found in one or more of the commits." >$GITHUB_STEP_SUMMARY
+          echo "" >>$GITHUB_STEP_SUMMARY
+          echo "Run \`git rebase --whitespace=fix ${lastcommit}\` and \`git push --force\` to correct the problem." >>$GITHUB_STEP_SUMMARY
+          for i in "${problems[@]}"
+          do
+            echo "${i}" >>$GITHUB_STEP_SUMMARY
+          done
+
           exit 2
         fi

-- 
gitgitgadget
Junio C Hamano· Dec 16, 2022, 10:06 UTC · re: Chris. Webster via GitGitGadget · lore

Re: [PATCH 1/2] Make `check-whitespace` failures more helpful

"Chris. Webster via GitGitGadget" <gitgitgadget@gmail.com> writes:
> Subject: Re: [PATCH 1/2] Make `check-whitespace` failures more helpful

People usually make changes to the system to make it "more useful" and/or "more helpful", and almost never to make it "less helpful". Phrases you would use to explain why the failures become more helpful with this change (compared to without) would help to promote it in the "git shortlog --no-merges" output for the next release. E.g. "make X failures stand out more", "make X failures gramatically correct", "show X failures more concisely", etc.

Show 10 quoted lines
> diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml
> index ad3466ad16e..3a99073bc33 100644
> --- a/.github/workflows/check-whitespace.yml
> +++ b/.github/workflows/check-whitespace.yml
> @@ -13,38 +13,57 @@ jobs:
>    check-whitespace:
>      runs-on: ubuntu-latest
>      steps:
> -    - uses: actions/checkout@v2
> +    - uses: actions/checkout@v3

I think we saw changes to upgrade actions/checkout@ in another topic, and it seems that we have missed this one even though we should have upgraded it the same way as other files in the same directory? Shouldn't this hunk be a separate topic on its own, or at least a separate patch on its own in the series?

Show 8 quoted lines
>        with:
>          fetch-depth: 0
>  
>      - name: git log --check
>        id: check_out
>        run: |
> -        log=
> +        problems=()

Is it safe to assume we run Bash here, or can GitHub start using other shells that lack the Bash-ism shell arrays and we should protect against such future?

I suspect that we are already depend on <<< Bash-ism, so one more dependency to Bash-ism is not a problem here? I dunno.

Thanks.
Chris Webster· Dec 20, 2022, 00:30 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] Make `check-whitespace` failures more helpful

On Fri, Dec 16, 2022 at 2:06 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 8 quoted lines
>
> People usually make changes to the system to make it "more useful"
> and/or "more helpful", and almost never to make it "less helpful".
> Phrases you would use to explain why the failures become more
> helpful with this change (compared to without) would help to promote
> it in the "git shortlog --no-merges" output for the next release.
> E.g. "make X failures stand out more", "make X failures gramatically
> correct", "show X failures more concisely", etc.
I will resend with a hopefully better explanation.
> directory?  Shouldn't this hunk be a separate topic on its own,
> or at least a separate patch on its own in the series?
It will now be a separate patch.
Show 6 quoted lines
> Is it safe to assume we run Bash here, or can GitHub start using
> other shells that lack the Bash-ism shell arrays and we should
> protect against such future?
>
> I suspect that we are already depend on <<< Bash-ism, so one more
> dependency to Bash-ism is not a problem here?  I dunno.

While GitHub could probably allow other shells to be used, changing the default would probably break a lot of things at this point.

Thanks for the feedback, ...chris.

Junio C Hamano· Dec 20, 2022, 01:36 UTC · re: Chris Webster · lore

Re: [PATCH 1/2] Make `check-whitespace` failures more helpful

Chris Webster <chris@webstech.net> writes:
Show 5 quoted lines
>> I suspect that we are already depend on <<< Bash-ism, so one more
>> dependency to Bash-ism is not a problem here?  I dunno.
>
> While GitHub could probably allow other shells to be used, changing
> the default would probably break a lot of things at this point.

Do we specifically ask for bash in our .github/ files? That would be perfectly acceptable. Then we only have to worry about their withdrawing support for bash which would never happen ;-)

Thanks.
Chris Webster· Dec 20, 2022, 05:50 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] Make `check-whitespace` failures more helpful

On Mon, Dec 19, 2022 at 5:36 PM Junio C Hamano <gitster@pobox.com> wrote:
> Do we specifically ask for bash in our .github/ files?  That would
> be perfectly acceptable.  Then we only have to worry about their
> withdrawing support for bash which would never happen ;-)

We use the default but the default can be changed https://docs.github.com/en/actions/using-jobs/setting-default-values-for-jobs. IOW we can always use bash if the default changes.

...chris.
Chris. Webster via GitGitGadget· Dec 20, 2022, 00:35 UTC · re: Chris. Webster via GitGitGadget · lore

[PATCH v2 0/3] Make check-whitespace failures more helpful

Add the errors to the job summary along with suggested commands to fix the problem. The commits and filenames are links.

This is for issue #1395. Sample job output [https://github.com/webstech/check-whitespace/actions/runs/3707382446]:

❌ A whitespace issue was found in one or more of the commits.
Run these commands to correct the problem:
1. git rebase --whitespace=fix aaa04a9
2. git push --force
Errors:
1. --- 5cd37f6 Remove annotations
   trailing.txt:4: trailing whitespace.
   +
   trailing.txt:2: new blank line at EOF.
Chris. Webster (3):
  ci (check-whitespace): suggest fixes for errors
  ci (check-whitespace): add links to job output
  ci (check-whitespace): move to actions/checkout@v3
 .github/workflows/check-whitespace.yml | 57 +++++++++++++++++++++-----
 1 file changed, 46 insertions(+), 11 deletions(-)
base-commit: 57e2c6ebbe7108b35ba30184dcbcb6c34c929ad8
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1444%2Fwebstech%2Fwhitespace-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1444/webstech/whitespace-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1444
Range-diff vs v1:
 1:  67f60e4e5cb ! 1:  a2b5f3e87d6 Make `check-whitespace` failures more helpful
     @@ Metadata
      Author: Chris. Webster <chris@webstech.net>
      
       ## Commit message ##
     -    Make `check-whitespace` failures more helpful
     +    ci (check-whitespace): suggest fixes for errors
      
     -    Add the errors to the job summary along with suggested
     -    commands to fix the problem.
     +    Make the errors more visible by adding them to the job summary and
     +    display the git commands that will usually fix the problem.
      
          Signed-off-by: Chris. Webster <chris@webstech.net>
      
       ## .github/workflows/check-whitespace.yml ##
      @@ .github/workflows/check-whitespace.yml: jobs:
     -   check-whitespace:
     -     runs-on: ubuntu-latest
     -     steps:
     --    - uses: actions/checkout@v2
     -+    - uses: actions/checkout@v3
     -       with:
     -         fetch-depth: 0
     - 
           - name: git log --check
             id: check_out
             run: |
 2:  cdc2b1aae81 ! 2:  342167ef5bd Improve check-whitespace output
     @@ Metadata
      Author: Chris. Webster <chris@webstech.net>
      
       ## Commit message ##
     -    Improve check-whitespace output
     +    ci (check-whitespace): add links to job output
      
          A message in the step log will refer to the Summary output.
      
     -    The job summary output now has links to the commits and files.
     +    The job summary output is using markdown to improve readability.  The
     +    git commands and commits with errors are now in ordered lists.
     +    Commits and files in error are links to the user's repository.
      
          Signed-off-by: Chris. Webster <chris@webstech.net>
      
 -:  ----------- > 3:  aa8cd940940 ci (check-whitespace): move to actions/checkout@v3
-- 
gitgitgadget
Chris. Webster via GitGitGadget· Dec 20, 2022, 00:35 UTC · re: Chris. Webster via GitGitGadget · lore

[PATCH v2 1/3] ci (check-whitespace): suggest fixes for errors

From: "Chris. Webster" <chris@webstech.net>

Make the errors more visible by adding them to the job summary and display the git commands that will usually fix the problem.

Signed-off-by: Chris. Webster <chris@webstech.net>
---
 .github/workflows/check-whitespace.yml | 37 +++++++++++++++++++-------
 1 file changed, 28 insertions(+), 9 deletions(-)
Show changes to .github/workflows/check-whitespace.yml +28 −9
diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml
index ad3466ad16e..a0871489b24 100644
--- a/.github/workflows/check-whitespace.yml
+++ b/.github/workflows/check-whitespace.yml
@@ -20,31 +20,50 @@ jobs:
     - name: git log --check
       id: check_out
       run: |
-        log=
+        problems=()
         commit=
-        while read dash etc
+        commitText=
+        lastcommit=
+        while read dash sha etc
         do
           case "${dash}" in
           "---")
-            commit="${etc}"
+            if test -z "${commit}"
+            then
+              lastcommit=${sha}
+            fi
+            commit="${sha}"
+            commitText="${sha} ${etc}"
             ;;
           "")
             ;;
           *)
             if test -n "${commit}"
             then
-              log="${log}\n${commit}"
+              problems+=("" "--- ${commitText}")
               echo ""
-              echo "--- ${commit}"
+              echo "--- ${commitText}"
+              commit=
             fi
-            commit=
-            log="${log}\n${dash} ${etc}"
-            echo "${dash} ${etc}"
+            problems+=("${dash} ${sha} ${etc}")
+            echo "${problems[-1]}"
             ;;
           esac
         done <<< $(git log --check --pretty=format:"---% h% s" ${{github.event.pull_request.base.sha}}..)
 
-        if test -n "${log}"
+        if test ${#problems[*]} -gt 0
         then
+          if test -z "${commit}"
+          then
+            lastcommit=${{github.event.pull_request.base.sha}}
+          fi
+          echo "A whitespace issue was found in one or more of the commits." >$GITHUB_STEP_SUMMARY
+          echo "" >>$GITHUB_STEP_SUMMARY
+          echo "Run \`git rebase --whitespace=fix ${lastcommit}\` and \`git push --force\` to correct the problem." >>$GITHUB_STEP_SUMMARY
+          for i in "${problems[@]}"
+          do
+            echo "${i}" >>$GITHUB_STEP_SUMMARY
+          done
+
           exit 2
         fi

-- 
gitgitgadget
Đoàn Trần Công Danh· Dec 20, 2022, 07:34 UTC · re: Chris. Webster via GitGitGadget · lore

Re: [PATCH v2 1/3] ci (check-whitespace): suggest fixes for errors

On 2022-12-20 00:35:45+0000, "Chris. Webster via GitGitGadget" <gitgitgadget@gmail.com> wrote:
Show 7 quoted lines
> From: "Chris. Webster" <chris@webstech.net>
> 
> Make the errors more visible by adding them to the job summary and
> display the git commands that will usually fix the problem.
> 
> Signed-off-by: Chris. Webster <chris@webstech.net>
> ---

I think this change is getting too long to be embeded in a yaml file. I think it's better to move the shell code into its own script, so we can have better code highlight in editor and a proper shebang (/bin/bash).

Show 60 quoted lines
>  .github/workflows/check-whitespace.yml | 37 +++++++++++++++++++-------
>  1 file changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml
> index ad3466ad16e..a0871489b24 100644
> --- a/.github/workflows/check-whitespace.yml
> +++ b/.github/workflows/check-whitespace.yml
> @@ -20,31 +20,50 @@ jobs:
>      - name: git log --check
>        id: check_out
>        run: |
> -        log=
> +        problems=()
>          commit=
> -        while read dash etc
> +        commitText=
> +        lastcommit=
> +        while read dash sha etc
>          do
>            case "${dash}" in
>            "---")
> -            commit="${etc}"
> +            if test -z "${commit}"
> +            then
> +              lastcommit=${sha}
> +            fi
> +            commit="${sha}"
> +            commitText="${sha} ${etc}"
>              ;;
>            "")
>              ;;
>            *)
>              if test -n "${commit}"
>              then
> -              log="${log}\n${commit}"
> +              problems+=("" "--- ${commitText}")
>                echo ""
> -              echo "--- ${commit}"
> +              echo "--- ${commitText}"
> +              commit=
>              fi
> -            commit=
> -            log="${log}\n${dash} ${etc}"
> -            echo "${dash} ${etc}"
> +            problems+=("${dash} ${sha} ${etc}")
> +            echo "${problems[-1]}"
>              ;;
>            esac
>          done <<< $(git log --check --pretty=format:"---% h% s" ${{github.event.pull_request.base.sha}}..)
>  
> -        if test -n "${log}"
> +        if test ${#problems[*]} -gt 0
>          then
> +          if test -z "${commit}"
> +          then
> +            lastcommit=${{github.event.pull_request.base.sha}}
> +          fi
> +          echo "A whitespace issue was found in one or more of the commits." >$GITHUB_STEP_SUMMARY
> +          echo "" >>$GITHUB_STEP_SUMMARY
> +          echo "Run \`git rebase --whitespace=fix ${lastcommit}\` and \`git push --force\` to correct the problem." >>$GITHUB_STEP_SUMMARY

When move this block into its own script, we can use single quote string here, too.

Show 10 quoted lines
> +          for i in "${problems[@]}"
> +          do
> +            echo "${i}" >>$GITHUB_STEP_SUMMARY
> +          done
> +
>            exit 2
>          fi
> -- 
> gitgitgadget
> 
-- 
Danh
Chris Webster· Dec 20, 2022, 19:55 UTC · re: Đoàn Trần Công Danh · lore

Re: [PATCH v2 1/3] ci (check-whitespace): suggest fixes for errors

> I think this change is getting too long to be embeded in a yaml file.
> I think it's better to move the shell code into its own script, so we
> can have better code highlight in editor and a proper shebang (/bin/bash).
That would need to be a separate patch?
> > +          echo "Run \`git rebase --whitespace=fix ${lastcommit}\` and \`git push --force\` to correct the problem." >>$GITHUB_STEP_SUMMARY
>
> When move this block into its own script, we can use single quote
> string here, too.
I am not sure what you mean.

Thanks for the review, ...chris.

Đoàn Trần Công Danh· Dec 21, 2022, 01:53 UTC · re: Chris Webster · lore

Re: [PATCH v2 1/3] ci (check-whitespace): suggest fixes for errors

On 2022-12-20 11:55:57-0800, Chris Webster <chris@webstech.net> wrote:
Show 5 quoted lines
> > I think this change is getting too long to be embeded in a yaml file.
> > I think it's better to move the shell code into its own script, so we
> > can have better code highlight in editor and a proper shebang (/bin/bash).
> 
> That would need to be a separate patch?

Yes, I think, a patch to move the whole block into a script, maybe in ci/ folder.

Show 7 quoted lines
> 
> > > +          echo "Run \`git rebase --whitespace=fix ${lastcommit}\` and \`git push --force\` to correct the problem." >>$GITHUB_STEP_SUMMARY
> >
> > When move this block into its own script, we can use single quote
> > string here, too.
> 
> I am not sure what you mean.
I mean we can write:
	echo 'Run `git rebase ...` to correct the problem'
With single quote, we need less escape.
-- 
Danh
Chris Webster· Dec 21, 2022, 06:08 UTC · re: Đoàn Trần Công Danh · lore

Re: [PATCH v2 1/3] ci (check-whitespace): suggest fixes for errors

On Tue, Dec 20, 2022 at 5:53 PM Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote:

> Yes, I think, a patch to move the whole block into a script, maybe in
> ci/ folder.

Maybe before the next patch or someone could create a check-whitespace workflow action. Can this patch move forward? A script would involve validating parameters or env variables that are just workflow context expressions now (ie more complexity).

Show 7 quoted lines
> > I am not sure what you mean.
>
> I mean we can write:
>
>         echo 'Run `git rebase ...` to correct the problem'
>
> With single quote, we need less escape.
What about ${lastcommit}?  Yes, there is more than one way to do it.

thanks, ...chris.

Đoàn Trần Công Danh· Dec 21, 2022, 13:46 UTC · re: Chris Webster · lore

Re: [PATCH v2 1/3] ci (check-whitespace): suggest fixes for errors

On 2022-12-20 22:08:58-0800, Chris Webster <chris@webstech.net> wrote:
Show 9 quoted lines
> On Tue, Dec 20, 2022 at 5:53 PM Đoàn Trần Công Danh
> <congdanhqx@gmail.com> wrote:
> > Yes, I think, a patch to move the whole block into a script, maybe in
> > ci/ folder.
> 
> Maybe before the next patch or someone could create a check-whitespace
> workflow action.  Can this patch move forward?  A script would involve
> validating parameters or env variables that are just workflow context
> expressions now (ie more complexity).

I would say, we can just check an environment variables specific to GitHub Action, and print a warning if it's missing. Other than that, just process as normal.

Show 9 quoted lines
> > > I am not sure what you mean.
> >
> > I mean we can write:
> >
> >         echo 'Run `git rebase ...` to correct the problem'
> >
> > With single quote, we need less escape.
> 
> What about ${lastcommit}?  Yes, there is more than one way to do it.
Ah, I misread that part. Sorry.
-- 
Danh
Chris. Webster via GitGitGadget· Dec 20, 2022, 00:35 UTC · re: Chris. Webster via GitGitGadget · lore

[PATCH v2 2/3] ci (check-whitespace): add links to job output

From: "Chris. Webster" <chris@webstech.net>
A message in the step log will refer to the Summary output.

The job summary output is using markdown to improve readability. The git commands and commits with errors are now in ordered lists. Commits and files in error are links to the user's repository.

Signed-off-by: Chris. Webster <chris@webstech.net>
---
 .github/workflows/check-whitespace.yml | 34 +++++++++++++++++++-------
 1 file changed, 25 insertions(+), 9 deletions(-)
Show changes to .github/workflows/check-whitespace.yml +25 −9
diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml
index a0871489b24..552894f736a 100644
--- a/.github/workflows/check-whitespace.yml
+++ b/.github/workflows/check-whitespace.yml
@@ -20,46 +20,62 @@ jobs:
     - name: git log --check
       id: check_out
       run: |
+        baseSha=${{github.event.pull_request.base.sha}}
         problems=()
         commit=
         commitText=
-        lastcommit=
+        commitTextmd=
+        goodparent=
         while read dash sha etc
         do
           case "${dash}" in
           "---")
             if test -z "${commit}"
             then
-              lastcommit=${sha}
+              goodparent=${sha}
             fi
             commit="${sha}"
             commitText="${sha} ${etc}"
+            commitTextmd="[${sha}](https://github.com/${{ github.repository }}/commit/${sha}) ${etc}"
             ;;
           "")
             ;;
           *)
             if test -n "${commit}"
             then
-              problems+=("" "--- ${commitText}")
+              problems+=("1) --- ${commitTextmd}")
               echo ""
               echo "--- ${commitText}"
               commit=
             fi
-            problems+=("${dash} ${sha} ${etc}")
-            echo "${problems[-1]}"
+            case "${dash}" in
+            *:[1-9]*:) # contains file and line number information
+              dashend=${dash#*:}
+              problems+=("[${dash}](https://github.com/${{ github.repository }}/blob/${{github.event.pull_request.head.ref}}/${dash%%:*}#L${dashend%:}) ${sha} ${etc}")
+              ;;
+            *)
+              problems+=("\`${dash} ${sha} ${etc}\`")
+              ;;
+            esac
+            echo "${dash} ${sha} ${etc}"
             ;;
           esac
-        done <<< $(git log --check --pretty=format:"---% h% s" ${{github.event.pull_request.base.sha}}..)
+        done <<< $(git log --check --pretty=format:"---% h% s" ${baseSha}..)
 
         if test ${#problems[*]} -gt 0
         then
           if test -z "${commit}"
           then
-            lastcommit=${{github.event.pull_request.base.sha}}
+            goodparent=${baseSha: 0:7}
           fi
-          echo "A whitespace issue was found in one or more of the commits." >$GITHUB_STEP_SUMMARY
+          echo "🛑 Please review the Summary output for further information."
+          echo "### :x: A whitespace issue was found in one or more of the commits." >$GITHUB_STEP_SUMMARY
           echo "" >>$GITHUB_STEP_SUMMARY
-          echo "Run \`git rebase --whitespace=fix ${lastcommit}\` and \`git push --force\` to correct the problem." >>$GITHUB_STEP_SUMMARY
+          echo "Run these commands to correct the problem:" >>$GITHUB_STEP_SUMMARY
+          echo "1. \`git rebase --whitespace=fix ${goodparent}\`" >>$GITHUB_STEP_SUMMARY
+          echo "1. \`git push --force\`" >>$GITHUB_STEP_SUMMARY
+          echo " " >>$GITHUB_STEP_SUMMARY
+          echo "Errors:" >>$GITHUB_STEP_SUMMARY
           for i in "${problems[@]}"
           do
             echo "${i}" >>$GITHUB_STEP_SUMMARY

-- 
gitgitgadget
Chris. Webster via GitGitGadget· Dec 20, 2022, 00:35 UTC · re: Chris. Webster via GitGitGadget · lore

[PATCH v2 3/3] ci (check-whitespace): move to actions/checkout@v3

From: "Chris. Webster" <chris@webstech.net>

Get rid of deprecation warnings in the CI runs. Also gets the latest security patches.

Signed-off-by: Chris. Webster <chris@webstech.net>
---
 .github/workflows/check-whitespace.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to .github/workflows/check-whitespace.yml +1 −1
diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml
index 552894f736a..da557fd5914 100644
--- a/.github/workflows/check-whitespace.yml
+++ b/.github/workflows/check-whitespace.yml
@@ -13,7 +13,7 @@ jobs:
   check-whitespace:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v3
       with:
         fetch-depth: 0
 

-- 
gitgitgadget

← back to recent threads