Re: Patch for fixing doc about some parameters.

From: Bo Peng <pengbo(at)sraoss(dot)co(dot)jp>
To: Koshino Taiki <koshino(at)sraoss(dot)co(dot)jp>, "pgpool-hackers(at)lists(dot)postgresql(dot)org" <pgpool-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Patch for fixing doc about some parameters.
Date: 2025-07-14 00:46:36
Message-ID: TYWP286MB2633DB2447A1E8CEBD96DAAAF254A@TYWP286MB2633.JPNP286.PROD.OUTLOOK.COM
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgpool-hackers

koshino-san,

Thank you for your patches.

--------------------------------------
v3-0001-Doc-fix-documentation-for-parameters-that-are-not.patch
--------------------------------------
It looks good.

--------------------------------------
v3-0001-Doc-fix-documentation-for-enum-parameters-reporte.patch
--------------------------------------
I think "backend_clustering_mode" should also be fixed. (sample config file and docs)

backend_clustering_mode = 'streaming_replication'

________________________________________
差出人: Koshino Taiki <koshino(at)sraoss(dot)co(dot)jp>
送信: 2025 年 7 月 9 日 (水曜日) 10:18
宛先: Bo Peng <pengbo(at)sraoss(dot)co(dot)jp>; pgpool-hackers(at)lists(dot)postgresql(dot)org <pgpool-hackers(at)lists(dot)postgresql(dot)org>
件名: Re: Patch for fixing doc about some parameters.

Peng-san.Thank you for reviewing.I improved patches.--------------------------------------v3-0001-Doc-fix-documentation-for-enum-parameters-reporte.patch.--------------------------------------For the following parameters:    log_standby_delay    wd_lifecheck_method    memqcache_method    disable_load_balance_on_writeI deleted '' on config file and document.------------------------------------------v3-0001-Doc-fix-documentation-for-parameters-that-are-not.patch------------------------------------------I fixed the configuration file to add the description such as "(change requires restart)".Taiki Koshino<koshino(at)sraoss(dot)co(dot)jp>SRA OSS K.K.TEL: 03-5979-2701 FAX: 03-5979-2702URL: https://www.sraoss.co.jp/差出人: Bo Peng <pengbo(at)sraoss(dot)co(dot)jp>送信日時: 2025年7月4日 16:59宛先: Koshino Taiki <koshino(at)sraoss(dot)co(dot)jp>; pgpool-hackers(at)lists(dot)postgresql(dot)org <pgpool-hackers(at)lists(dot)postgresql(dot)org>件名: Re: Patch for fixing doc about some parameters.  Koshino-san,Thank you for your patch.I have reviewed your patch and have a few comments.--------------------------------------v2-0001-Doc-fix-documentation-for-enum-parameters-reporte.patch.--------------------------------------For the following parameters:    log_standby_delay    wd_lifecheck_method    memqcache_method    disable_load_balance_on_writeIn the documentation, their types have been correctly changed to enum, which is good.However, in both the configuration file examples and the parameter description sections,these values are still written as strings (enclosed in single quotes).For example:(config file)#wd_lifecheck_method = 'heartbeat'(Document)wd_lifecheck_method (enum)    Specifies the method of life check. This can be either of 'heartbeat' (default), 'query' or 'external'. This is no big issue since Pgpool-II accepts string values (e.g. 'foo') even for enum parameters.However, it's a bit confusing, so I think it should be fixed.For example:#wd_lifecheck_method = heartbeat                                                   # Method of watchdog lifecheck (heartbeat or query or external)wd_lifecheck_method (enum)    Specifies the method of life check. This can be either of <literal>heartbeat</literal> (default), <literal>query</literal> or <literal>external</literal>. ------------------------------------------v2-0001-Doc-fix-documentation-for-parameters-that-are-not.patch------------------------------------------I think we should also fix the configuration file to add the description such as "(change requires restart)".For example:#authentication_timeout = 1min                                   # Delay in seconds to complete client authentication                                   # 0 means no timeout.                                   # (change requires restart)________________________________________差出人: Koshino Taiki <koshino(at)sraoss(dot)co(dot)jp>送信: 2025 年 7 月 4 日 (金曜日) 15:15宛先: pgpool-hackers(at)lists(dot)postgresql(dot)org <pgpool-hackers(at)lists(dot)postgresql(dot)org>件名: Re: Patch for fixing doc about some parameters.Thank you for reviewing.I removed the whitespace.And I changed the commit title to"Doc: fix documentation for enum parameters reported asstrings""Doc: fix documentation for parameters that are notreflected by reload."After receiving feedback from Peng-san, I will create a patch again.Taiki Koshino<koshino(at)sraoss(dot)co(dot)jp>SRA OSS K.K.TEL: 03-5979-2701 FAX: 03-5979-2702URL: https://www.sraoss.co.jp/差出人: Tatsuo Ishii <ishii(at)postgresql(dot)org>送信日時: 2025年7月2日 20:27宛先: Koshino Taiki <koshino(at)sraoss(dot)co(dot)jp>CC: pgpool-hackers(at)lists(dot)postgresql(dot)org <pgpool-hackers(at)lists(dot)postgresql(dot)org>件名: Re: Patch for fixing doc about some parameters. > Fixed mistakes in the documentation for parameters.>> "v1-0001-Fixed-documentation-for-parameters-that-are-not-r.patch"> is a patch for ticket #10954> "Test all the configuration parameters if reload is required."There's one trainling whitespace.$ git apply ~/v1-0001-Fixed-documentation-for-parameters-that-are-not-r.patch/home/t-ishii/v1-0001-Fixed-documentation-for-parameters-that-are-not-r.patch:39: trailing whitespace.      このパラメータはサーバ起動時にのみ設定可能です。warning: 1 line adds whitespace errors.> Subject: [PATCH v2] Fixed documentation for parameters that are not reflected by reload.This comes from the commit title "Fixed documentation for parametersthat are not reflected by reload." We usually add "Doc:" prefix to thecommit header if the changes are only for documentations. Also "Fixed"seems unnatural as a commit header. Instead use "fix". So somethinglike "Doc: fix documentation for parameters that are not reflected byreload." is better,> "v1-0001-Fix-documentation-for-enum-parameters-reported-as"> is for ticket #8397Please add "Doc:" prefix to the commit title as well.#8397 is an internal number and you'd better to remove it.Other than that, the patch looks good to me.Best regards,--Tatsuo IshiiSRA OSS K.K.English: http://www.sraoss.co.jp/index_en/Japanese:http://www.sraoss.co.jp

In response to

Responses

Browse pgpool-hackers by date

  From Date Subject
Next Message Koshino Taiki 2025-07-14 02:05:05 Re: Patch for fixing doc about some parameters.
Previous Message Tatsuo Ishii 2025-07-10 10:38:13 Re: Feature: implement NegotiateProtocolVersion message