On Tue, Sep 28, 2021 at 8:06 AM Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:
> I thought mentioning the unit in milliseconds and the example in
> seconds would confuse the user, so I changed the example to
> milliseconds.The message behind the above description looks good to me
> however I feel some sentences can be explained in less words. The
> information related to the units is missing and I feel it should be
> mentioned in the document. The example says, if the setting has the
> default value of 10 seconds, then it explains the behaviour. I feel it
> may not be the default value, it can be any value set by the user. So
> mentioning 'default' in the example does not look good to me. I feel
> we just have to mention "if this setting is set to the value of 10
> seconds". Below is the modified version of the above information.
It is common to mention what the default is as part of the
documentation of a GUC. I don't see why this one should be an
exception, especially since not mentioning it reduces the length of
the documentation by exactly one word.
I don't mind the sentence you added at the end to clarify the default
units, but the way you've rewritten the first sentence makes it, in my
opinion, much less clear.
> I had added additional code to check the value of the
> 'log_startup_progress_interval' variable and disable the feature in
> case of -1 in the earlier versions of the patch (Specifically
> v9.patch). There was a review comment for v9 patch and it resulted in
> major refactoring of the patch.
...
> The answer for the question of "I don't understand why you posted the
> previous version of the patch without testing that it works" is, for
> the value of -1, the above description was my understanding and for
> the value of 0, the older versions of the patch was behaving as
> documented. But with the later versions the behaviour got changed and
> I missed to modify the documentation. So I modified it in the last
> version.
v9 was posted on August 3rd. I told you that it wasn't working on
September 23rd. You posted a new version that still does not work on
September 27th. I think you should have tested each version of your
patch before posting it, and especially after any major refactorings.
And if for whatever reason you didn't, then certainly after I told you
on September 23rd that it didn't work, I think you should have fixed
it before posting a new version.
--
Robert Haas
EDB: http://www.enterprisedb.com