On Tue, 25 Jul 2023 at 19:36, Junio C Hamano <gitster@xxxxxxxxx> wrote: > > M Hickford <mirth.hickford@xxxxxxxxx> writes: > > >> * mh/credential-erase-improvements-more (2023-06-24) 2 commits > >> - credential/wincred: erase matching creds only > >> - credential/libsecret: erase matching creds only > >> > >> Needs review. > >> source: <pull.1529.git.git.1687596777147.gitgitgadget@xxxxxxxxx> > > > > Hi. Is anyone able to help review these changes? > > > > https://lore.kernel.org/git/pull.1529.git.git.1687596777147.gitgitgadget@xxxxxxxxx/ > > https://lore.kernel.org/git/pull.1527.git.git.1687591293705.gitgitgadget@xxxxxxxxx/ > > Thanks for pinging. One thing that may help (both patches, my > understanding is that they are of the same spirit, just one is for > libsecret while the other one is for wincred) is to describe the > problem the patches attempt to address a bit more. For example, > in one of them: > > Fix test "helper ... does not erase a password distinct from input" > introduced in aeb21ce22e (credential: avoid erasing distinct password, > 2023-06-13) > > we can read from the above proposed log message that it is a "fix" > to some bug, and that the "bug" was introduced by the named commit, > but there are a few things that it does not explain, that could have > helped readers to convince themselves that the changes in the patches > are addressing the right problems and solving them in the right > way. For example, > > * how does the "bug" manifest itself in an observable way to the > end-users? "When they do X, they expect Y to happen, but instead > Z happens, and doing Z breaks expectation of users expecting Y in > this (W) way." > > * what was wrong in the code that led to the "bug"? Was it testing > a wrong condition before making a call to some system service? > Was the condition it checked correct but it made a call in a > wrong way (and if so how)? > > Thanks. > Thanks for the suggestions. I'll send a patch v2 with an updated commit message that tries to answer these questions.
