On Wed, Oct 22, 2014 at 11:08:22AM +0200, Paolo Bonzini wrote:
> The list emitted by --git-fallback often leads inexperienced contributors
> to add pointless CCs. While not discouraging usage of --git-fallback,
> we want to warn the contributors about using their common sense.
>
> So, default to *not* enabling --git-fallback, but print a message if
> none of the files has a match against MAINTAINERS. Of course the
> message is hidden by --no-git-fallback.
>
> Examples:
>
> 1) No maintainer for all specified files, print message:
>
> $ scripts/get_maintainer.pl -f util/cutils.c
> No maintainers found.
> You may want to try --git-fallback to find recent contributors.
> Do not blindly cc: them on patches! Use common sense.
>
Does it make sense for util/cutils.c?
I doubt it, so we are just giving useless advice?
--git-blame might be a better fallback here?
How about an entry in MAINTAINERS to trigger git-blame?
> 2) No maintainer for some of the specified files, behave entirely
> as if the user specified --no-git-fallback.
>
> $ scripts/get_maintainer.pl -f util/cutils.c hw/ide/core.c
> Kevin Wolf <kw...@redhat.com> (odd fixer:IDE)
> Stefan Hajnoczi <stefa...@redhat.com> (odd fixer:IDE)
>
> 3) Explicit disable, do not print message:
>
> $ scripts/get_maintainer.pl -f util/cutils.c --no-git-fallback
> $ echo $?
> 1
>
> Suggested-by: Markus Armbruster <arm...@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> scripts/get_maintainer.pl | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index 7c6d186..c8537fd 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -28,7 +28,7 @@ my $email_git = 0;
> my $email_git_all_signature_types = 0;
> my $email_git_blame = 0;
> my $email_git_blame_signatures = 1;
> -my $email_git_fallback = 1;
> +my $email_git_fallback = undef;
> my $email_git_min_signatures = 1;
> my $email_git_max_maintainers = 5;
> my $email_git_min_percent = 5;
> @@ -235,6 +235,7 @@ if (-t STDIN && !@ARGV) {
> }
>
> $output_multiline = 0 if ($output_separator ne ", ");
> +$email_git_fallback = 1 if ($interactive && ! defined $email_git_fallback);
> $output_rolestats = 1 if ($interactive);
> $output_roles = 1 if ($output_rolestats);
>
> @@ -633,6 +634,14 @@ sub get_maintainers {
> }
>
> if ($email) {
> + if (@email_to == 0 && @list_to == 0 &&
> + ! $email_git && ! $email_git_blame && ! defined
> $email_git_fallback) {
> + print STDERR "No maintainers found.\n";
> + print STDERR "You may want to try --git-fallback to find recent
> contributors.\n";
So let's just do this for the user?
> + print STDERR "Do not blindly cc: them on patches! Use common
> sense.\n";
Can we do better than "Use common sense"?
I doubt such advice accomplishes much.
> + }
> +
> + $email_git_fallback = 0 if ! defined $email_git_fallback;
> foreach my $file (@files) {
> if ($email_git || ($email_git_fallback &&
> !$exact_pattern_match_hash{$file})) {
> @@ -711,7 +720,7 @@ MAINTAINER field selection options:
> --git => include recent git \*-by: signers
> --git-all-signature-types => include signers regardless of signature type
> or use only ${signature_pattern} signers (default:
> $email_git_all_signature_types)
> - --git-fallback => use git when no exact MAINTAINERS pattern (default:
> $email_git_fallback)
> + --git-fallback => use git when no exact MAINTAINERS pattern (default:
> same value as --interactive)
> --git-chief-penguins => include ${penguin_chiefs}
> --git-min-signatures => number of signatures required (default:
> $email_git_min_signatures)
> --git-max-maintainers => maximum maintainers to add (default:
> $email_git_max_maintainers)
> --
> 1.8.3.1
