On Wed, Mar 27, 2024 at 6:24 PM Bruce Momjian <br...@momjian.us> wrote: > Please ignore my complaints, and my apologies. > > As far as the GUC change, let's just be careful since we have a bad > history of pushing things near the end that we regret. I am not saying > that would be this feature, but let's be careful.
Even if what I had pushed was the patch itself, so what? This patch has been sitting around, largely unchanged, for six months. There has been plenty of time for wordsmithing the documentation, yet nobody got interested in doing it until I expressed interest in committing the patch. Meanwhile, there are over 100 other patches that no committer is paying attention to right now, some of which could probably really benefit from some wordsmithing of the documentation. It drives me insane that this is the patch everyone is getting worked up about. This is a 27-line code change that does something many people want, and we're acting like the future of the project depends on it. Both I and others have committed thousands of lines of new code over the last few months that could easily be full of bugs that will eat your data without nearly the scrutiny that this patch is getting. To be honest, I had every intention of pushing the main patch right after I pushed that preliminary patch, but I stopped because I saw you had emailed the thread. I'm pretty sure that I would have missed the fact that the documentation hadn't been properly updated for the fact that the sense of the GUC had been inverted. That would have been embarrassing, and I would have had to push a follow-up commit to fix that. But no real harm would have been done, except that somebody surely would have seized on my mistake as proof that this patch wasn't ready to be committed and that I was being irresponsible and inconsiderate by pushing forward with it, which is a garbage argument. Committers make mistakes like that all the time, every week, even every day. It doesn't mean that they're bad committers, and it doesn't mean that the patches suck. Some of the patches that get committed do suck, but it's not because there are a few words wrong in the documentation. Let's please, please stop pretending like this patch is somehow deserving of special scrutiny. There's barely even anything to scrutinize. It's literally if (!variable) ereport(...) plus some boilerplate and docs. I entirely agree that, because of the risk of someone filing a bogus CVE, the docs do need to be carefully worded. But, I'm going to be honest: I feel completely confident in my ability to review a patch well enough to know whether the documentation for a single test-and-ereport has been done up to project standard. It saddens and frustrates me that you don't seem to agree. -- Robert Haas EDB: http://www.enterprisedb.com