| Message ID | a2b5f3e87d6ef62d8005cff5568ad3afc4af3771.1671496548.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 288e3c4e3b54b9712c10b6cee51c908664471393 |
| Headers | show |
| Series | Make check-whitespace failures more helpful | expand |
On 2022-12-20 00:35:45+0000, "Chris. Webster via GitGitGadget" <gitgitgadget@gmail.com> wrote: > 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). > .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. > + for i in "${problems[@]}" > + do > + echo "${i}" >>$GITHUB_STEP_SUMMARY > + done > + > exit 2 > fi > -- > gitgitgadget >
> 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.
On 2022-12-20 11:55:57-0800, Chris Webster <chris@webstech.net> wrote: > > 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. > > > > + 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.
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 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.
On 2022-12-20 22:08:58-0800, Chris Webster <chris@webstech.net> wrote: > 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. > > > 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.
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