Thanks. Looks pretty good. My review comments:
> + for c in "${force[@]}"; do
> + if [ "$c" = "$check" ]; then
> + check_is_forced=true
> + break
> + fi
> + done
It is possible, including here I think, to do something like this:
case " ${force[*]} " in
*" $check "*) check_is_forced=true ;;
esac
If you make force be a list of ,-terminated items rather than an
array, the user can say --force=foo,bar and the case stunt I propose
above (with spaced replaced with appropriate commas) will still do
nicely.
Up to you.
> + '--force')
> + case "$2" in
> + 'suite'|'upstream-nonancestor'|'unreleased'|'dgit-view')
> + force+=("$2") ;;
> + '')
> + force_all=true ;;
> + *)
> + fail "invalid name of check to force: $2" ;;
This is not correct. Unrecognised force options should be ignored, so
that if we introduce a new check you can force it in a way that works
with old git-debpush.
We should perhaps add
--force=no-such-force-option
to an arbitrary one of the runes in tagupl.
> + fail_check "unreleased" "UNRELEASED changelog"
I wouldn't quote the check name personally. After all we are
certainly not going to introduce check names that contain shell
metacharacters...
Up to you.
> +if $failed_check; then
> + # We don't mention the --force=check options here as those are
> + # mainly for use by scripts, or when you already know what check
> + # is going to fail before you invoke git-debpush. Keep the
> + # script's terminal output as simple as possible. No "see the
> + # manpage"!
AFAICT nothing prints the name of the check that failed. Can we
please print it *somewhere* ? Ideally in the error message printed by
fail_check. Not printing it makes this new feature much less useable.
How about adding `[--force=THING]' to the per-failure error message
and doing this:
- fail "some checks failed; you can override with --force"
+ fail "some check(s) failed; say just --force to override them all"
or something ?
Ian.
--
Ian Jackson <[email protected]> These opinions are my own.
If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.
