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

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

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.