Re: [pve-devel] [PATCH access-control v2 1/3] authenticate_2nd_new: only lock tfa config for recovery keys

[Dominik Csapak <d.csapak@proxmox.com>]

On 10/21/22 13:29, Thomas Lamprecht wrote:
> Can we *please* get sane commit subject for *human* consumption?!
> 
> The worst one, that really triggers me:
>> authenticate_user: pass undef instead of empty $tfa_challenge to authenticate_2nd_new
> Instead of a high level human overview it gives basically another almost
> machine readable diff of the lower level changes. So there's close to zero
> of useful higher level and/or _new_ info in there, but sure makes one stop
> to parse on seeing this in some log.
> 
> Copying the method name (nor file module name!) is basically never a good
> idea, especially as tag and especially for (admin) user facing changes - it
> can naturally be OK for library stuff (i.e., dev-only facing changes), but
> even then seldom as start `<tag>:`...
> 
> following would allow for a quicker to read, and (granted, slightly
> subjective) better high level understanding of the commits, thus better
> categorizing for when skimming through the log, e.g., in search of relevant
> changes due to some new/questionable/.. behavior; besides that it makes
> d/changelog writing easier.
> 
> * tfa: only lock config for recovery keys on authentication
> * tfa: rename outdated $otp variable to $tfa_response
>    (this is internal and won't ever make it in the d/changelog so only dev
>    readability matters)
> * auth: drop passing bogus challenge variable when checking first factor
>    (also internally, dev oriented)
> 
> This should be also verified and either amened or commented on by the
> maintainer (planning) pushing a commit/series - while its obviously 1) hard
> and 2) a lot overhead to have a very strict rule book that's fine for a
> partially subjective thing like this, it should be that hard to detect the
> obvious cases, at least for user facing changes.
> 
> May look like a small thing to "rant" on, but I spend a lot of time in git
> log et al. and copy pasted method/file names without simple concise tagging
> and for-human info can make it much harder with no benefit for anyone.
> 
> Naturally applies to all devs/maintainers not just those in To/Cc here.

yes, you're completely right of course.

i don't think that it's a 'small thing to rant on' at all, i often
looked for commits where a better subject would have made it much more
obvious what is happening and would have been easier to identify

i'll use better commit subjects in the future.