Rename --registry-scanning to --registry-disable-scanning & keep it independent from --git-readonly

[2opremio]

Fixes #2790

[squaremo]

You didn't fix the boolean-flag-defaulting-to-true problem from #2790.

[2opremio]

You didn't fix the boolean-flag-defaulting-to-true problem from #2790.

I don't really see it as a problem, but I can change it if you see it as a showstopper.

[squaremo]

You didn't fix the boolean-flag-defaulting-to-true problem from #2790.

I don't really see it as a problem, but I can change it if you see it as a showstopper.

Well, perhaps either implement it as suggested in the issue, or mount an argument against doing so. My argument is essentially that it's convention to have boolean flags default to false, so having one that defaults to true will surprise some people; and, secondarily, it's a trap for future players, since boolean values (in libraries, structs, variables) default to false, and eventually someone will forget that this one flag is different, and produce unexpected results. There would probably not be a lot of fallout, but why not avoid it if you can?

[2opremio]

My only argument is that double negatives are not intuitive.

…

On Mon, Feb 3, 2020, 18:45 Michael Bridgen ***@***.***> wrote: You didn't fix the boolean-flag-defaulting-to-true problem from #2790 <#2790>. I don't really see it as a problem, but I can change it if you see it as a showstopper. Well, perhaps either implement it as suggested in the issue, or mount an argument against doing so. My argument is essentially that it's convention to have boolean flags default to false, so having one that defaults to true will surprise some people; and, secondarily, it's a trap for future players, since boolean values (in libraries, structs, variables) default to false, and eventually someone will forget that this one flag is different, and produce unexpected results. There would probably *not* be a lot of fallout, but why not avoid it if you can? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2813?email_source=notifications&email_token=AASA4JDKHD5BG3P3GUJPLPTRBBJ4XA5CNFSM4KPIC352YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKUX33Y#issuecomment-581533167>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASA4JHNVQM3EZI3CXREYF3RBBJ4XANCNFSM4KPIC35Q> .

[squaremo]

My only argument is that double negatives are not intuitive.

Double negatives don't arise -- you either supply

--disable-registry-scanning

to turn scanning off, or omit that flag to let it default to false, i.e., to keep the status quo.

(If the status quo were that registry scanning was off and you had to explicitly switch it on, then the recommendation would be to have --registry-scanning.)

[2opremio]

Fair enough. I will change it later today

…

On Tue, Feb 4, 2020, 11:43 Michael Bridgen ***@***.***> wrote: My only argument is that double negatives are not intuitive. Double negatives don't arise -- you either supply --disable-registry-scanning to turn scanning off, or omit that flag to let it default to false, i.e., to keep the status quo. (If the status quo were that registry scanning was *off* and you had to explicitly switch it *on*, then the recommendation would be to have --registry-scanning.) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2813?email_source=notifications&email_token=AASA4JDUM6H72P7TKFBGJCTRBFBFTA5CNFSM4KPIC352YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKXEXKA#issuecomment-581847976>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASA4JDGSG4OY5DOJQQZVYDRBFBFTANCNFSM4KPIC35Q> .

[2opremio]

The e2e tests caught a pretty basic error which unit tests wouldn't had been able to catch (it happened in the initialization of main()). I am so happy about this :)

[2opremio]

Fair enough. I will change it later today

@squaremo I am changing the flag now and found another reason for not having the --disable prefix: consistency. All the other registry-related flags start with --registry

I will continue changing it anyhow since you fell strongly in favor of it. Just leaving it for the record.

[squaremo]

The e2e tests caught a pretty basic error which unit tests wouldn't had been able to catch

Also a wonderful illustration of why not to introduce a boolean field which needs to be true for behaviour to stay the same. (We've all done it!)

[squaremo]

another reason for not having the --disable prefix: consistency. All the other registry-related flags start with --registry

You can change the name, it's the default value I feel strongly about. --registry-disable-scanning, e.g.

[2opremio]

Ah, that's better.

[2opremio]

Also a wonderful illustration of why not to introduce a boolean field which needs to be true for behaviour to stay the same. (We've all done it!)

For the record, I inherited that part of the code :)

[2opremio]

@squaremo I think I've addressed all your comments, PTAL

[squaremo]

Looks good to me -- thank you for patiently following this up