Justin Tobler <jltobler@xxxxxxxxx> writes: > On 24/07/08 11:23AM, Karthik Nayak wrote: >> The 'check-whitespace' CI script exists gracefully if no base commit is >> provided or if an invalid revision is provided. This is not good because >> if a particular CI provides an incorrect base_commit, it would fail >> successfully. > > s/exists/exits > > If no base commit is provided, we already fail. Here is an example > GitLab CI job demonstrating this: > https://gitlab.com/gitlab-org/git/-/jobs/7289543498#L2370 > > If the base commit does not exist though, it currently prints that error occured > but still exits with 0. Makes sense to update and fail the job accordingly. > This example is running on the 'edb990d9', whose parent's '8d9bcf0a' parent '57fdb00c' contains my changes, specifically the `|| test -z "$1"` section. You can check this on master locally though. $ ./ci/check-whitespace.sh "" fatal: ..: '..' is outside repository at '/home/karthik/git' $ echo $? 0 or for invalid value: $ ./ci/check-whitespace.sh "random" fatal: ambiguous argument 'random..': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git <command> [<revision>...] -- [<file>...]' $ echo $status 0 [snip] > > I might be misunderstanding, but this additional check seems redundant to me. > It is required, as commented above >> then >> echo "USAGE: $0 <BASE_COMMIT> [<OUTPUT_FILE> <URL>]" >> exit 1 >> fi >> >> +gitLogOutput=$(git log --check --pretty=format:"---% h% s" "${baseCommit}"..) >> +if test $? -ne 0 >> +then >> + echo -n $gitLogOutput >> + exit 1 >> +fi >> + >> problems=() >> commit= >> commitText= >> @@ -58,7 +65,7 @@ do >> echo "${dash} ${sha} ${etc}" >> ;; >> esac >> -done <<< "$(git log --check --pretty=format:"---% h% s" "${baseCommit}"..)" >> +done <<< "$gitLogOutput" >> >> if test ${#problems[*]} -gt 0 >> then >> @@ -67,7 +74,7 @@ then >> goodParent=${baseCommit: 0:7} >> fi >> >> - echo "A whitespace issue was found in onen of more of the commits." >> + echo "A whitespace issue was found in one of more of the commits." > There is another preexisting typo: > > s/one of/one or/ > Thanks. will add
Attachment:
signature.asc
Description: PGP signature
