Re: [PATCH] send-email: error out when relogin delay is missing

[Eric Sunshine]

On Thu, Feb 8, 2018 at 1:21 PM, Stefan Beller  wrote:
> On Thu, Feb 8, 2018 at 12:08 AM, Eric Sunshine  
> wrote:
>> On Wed, Feb 7, 2018 at 6:43 PM, Stefan Beller  wrote:
>>> +die __("When a batch size is given, the relogin delay must be set\n")
>>> +   if defined $relogin_delay and not defined $batch_size;
>>
>> This only makes sense is 'batch-size' is specified but not 'relogin'.
>> If the other way around, then the error is confusing. How about this
>> instead?
>> "--batch-size and --relogin must be specified together"
>> ...or something.
>
> I like this for its expressiveness as it would have helped me a lot.
> I dislike this because it is incorrect when you use the config options
> instead of command line arguments.

Perhaps:

"`batch-size` and `relogin` must be specified together
  (via command-line or configuration option)"

Re: [PATCH] send-email: error out when relogin delay is missing

[Stefan Beller]

On Thu, Feb 8, 2018 at 12:08 AM, Eric Sunshine  wrote:
> On Wed, Feb 7, 2018 at 6:43 PM, Stefan Beller  wrote:
>> [...]
>> Error out for now instead of potentially confusing the user.
>> As 5453b83bdf (send-email: --batch-size to work around some SMTP
>> server limit, 2017-05-21) lays out, we rather want to not have this
>> interface anyway and would rather want to react on the server throttling
>> dynamically.
>>
>> Signed-off-by: Stefan Beller 
>> ---
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> @@ -379,6 +379,9 @@ unless ($rc) {
>> +die __("When a batch size is given, the relogin delay must be set\n")
>> +   if defined $relogin_delay and not defined $batch_size;
>
> This only makes sense is 'batch-size' is specified but not 'relogin'.
> If the other way around, then the error is confusing. How about this
> instead?
>
> "--batch-size and --relogin must be specified together"
>
> ...or something.

I like this for its expressiveness as it would have helped me a lot.
I dislike this because it is incorrect when you use the config options
instead of command line arguments.

Stefan

Re: [PATCH] send-email: error out when relogin delay is missing

[Eric Sunshine]

On Wed, Feb 7, 2018 at 6:43 PM, Stefan Beller  wrote:
> [...]
> Error out for now instead of potentially confusing the user.
> As 5453b83bdf (send-email: --batch-size to work around some SMTP
> server limit, 2017-05-21) lays out, we rather want to not have this
> interface anyway and would rather want to react on the server throttling
> dynamically.
>
> Signed-off-by: Stefan Beller 
> ---
> diff --git a/git-send-email.perl b/git-send-email.perl
> @@ -379,6 +379,9 @@ unless ($rc) {
> +die __("When a batch size is given, the relogin delay must be set\n")
> +   if defined $relogin_delay and not defined $batch_size;

This only makes sense is 'batch-size' is specified but not 'relogin'.
If the other way around, then the error is confusing. How about this
instead?

"--batch-size and --relogin must be specified together"

...or something.

Re: [PATCH] send-email: error out when relogin delay is missing

[xiaoqiang zhao]

> 在 2018年2月8日，上午7:43，Stefan Beller  写道：
> 
> +die __("When a batch size is given, the relogin delay must be set\n")
> +if defined $relogin_delay and not defined $batch_size;
> +
 
According the code, maybe you want to say “When relogin delay is given, a batch 
size must be set “ ?