On Wed, Mar 27, 2024 at 6:24 PM Bruce Momjian <bruce@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
