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

Dominik Csapak d.csapak at proxmox.com
Fri Oct 21 13:38:17 CEST 2022

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.

More information about the pve-devel mailing list