Hi Corinna, On Thu, 18 Dec 2025, Corinna Vinschen wrote:
> On Dec 18 09:08, Johannes Schindelin wrote: > > As I have debugged Cygwin bugs quite often in the past, I can tell you > > that it is quite frustrating to end up finding commits that leave > > everything unclear, whose diffs are too large to find any obvious > > bugs. > > You'll come across them especially back in CVS times. CVS didn't > support patchsets along the lines of "one patch per problem" and the > blessed way of CVS ChangeLog entries didn't support the notion of > explaining at great length why a patch was necessary but rather only > what a patch is doing where. Whoever had the idea how a CVS ChangeLog > entry is supposed to look like, got it wrong, and thousands of > developers followed suit, including me. Example: > > 2015-04-22 Corinna Vinschen <[email protected]> > > * fhandler_tty.cc (fhandler_pty_slave::fch_close_handles): Don't close > handles not opened via fhandler_pty_slave::fch_open_handles. > > I have no idea why this was necessary at the time and we have the entire > 20 years of history since 1995 documented this way. > > And, we have a couple of years using git, in which we had to wrap our > heads around the new-fangled way to document patches in git commit > messages. And even if some of our commit messages might still be > lacking, they are much better than some other git entries in projects, > which don't seem to have learned how useful git commit messages can > be, if they try to describe the problem they are solving and the way > how to solve it. Example from FreeBSD in 2025: > > cxgbe: Stop using bus_space_tag/handle directly > > Reviewed by: np, imp > Sponsored by: [...] > Differential Revision: https://reviews.freebsd.org/D53030 > > That's it. They are getting better, there are actually some git commit > messages explaining the problem they are trying to solve and how, but > it's not standard throughout. And let me state how grateful I am about them getting better! > What I'm trying to say is this: > > It doesn't make sense to reflect on the past. > > We can do better, but for that, let's please stay professional and just > review the patch. Tell the patch contributor what you think is wrong > and wait for v2. Rinse and repeat with v3, v4, ... until you're > satisfied. > > Johannes, I think we all understood what you're trying to say, but this > is *not* how to say it. Yes, you're right, my behavior was unacceptable. I apologize! > Review the patch, criticize the patch (including it's commit message) > and stick to the patch. > > If you have good arguments or can point out bugs or problems, I and, I'm > sure, Takashi and others as well, will change their patch accordingly, > or even drop the patch if it's incorrect. But: don't tell people *how* > to spend their *voluntary* time. Stick to the patch, it's what counts. > > As a side-note, I lost track of the pty patches which were already GTG > and the patches which are not. Takashi, can you please push what's > already not in question and make a clean restart with a new thread on > this list? I'm really confused, and maybe I'm not alone... I could not pay attention to anything on the list for the past 6h because my laptop lost its ability to connect to the internet, and I worked frantically to fix it ever since. My take was that Takashi's https://inbox.sourceware.org/cygwin-patches/[email protected]/ is in a pretty decent shape. If it was up to me, I'd take it as-is (in the interest of moving on, and taking time off over the holidays). That's what I did in https://github.com/msys2/msys2-runtime/pull/322 and in https://github.com/git-for-windows/msys2-runtime/pull/122: I replaced my patches with Takashi's v4. Ciao, Johannes
