On Tue, Sep 28, 2021 at 8:06 AM Nitin Jadhav <nitinjadhavpostg...@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
