> I agree a proposal would be better before adding a PR. But the document part must be a part of such a proposal.
Make sense. It looks like we should have a checklist for the proposal. The documentation changes should be listed in the proposal. > Can the PR template/GitHub process check that if either the api changes and doc-required are checked both are checked with textual information provided? It's a good idea. I haven't tried, but it looks like it's possible. We have this list: ``` ### Does this pull request potentially affect one of the following parts: *If the box was checked, please highlight the changes* - [ ] Dependencies (add or upgrade a dependency) - [ ] The public API - [ ] The schema - [ ] The default values of configurations - [ ] The binary protocol - [ ] The REST endpoints - [ ] The admin CLI options - [ ] Anything that affects deployment ``` And the CI can try to add labels `doc required` or `wants/proposal` according to the list selections. And we can add `The metrics` item to the list. Thanks, Penghui On Wed, Dec 7, 2022 at 1:52 PM Dave Fisher <[email protected]> wrote: > > > Sent from my iPhone > > > On Dec 6, 2022, at 9:45 PM, Yunze Xu <[email protected]> > wrote: > > > > Hi Penghui, > > > >> But maybe some are missed. > > > > That's the point. Each PR that adds or modifies a metric item must be > > labeled with "doc-required" and the related documents should be added. > > However, these PRs are nearly all labeled with "doc-not-needed". > > > > I agree a proposal would be better before adding a PR. But the > > document part must be a part of such a proposal. > > Can the PR template/GitHub process check that if either the api changes > and doc-required are checked both are checked with textual information > provided? > > Best, > Dave > > > > Thanks, > > Yunze > > > >> On Wed, Dec 7, 2022 at 11:48 AM PengHui Li <[email protected]> wrote: > >> > >> Hi Yunze, > >> > >> All the metrics are listed here > >> https://pulsar.apache.org/docs/2.10.x/reference-metrics/ > >> > >> But maybe some are missed. > >> > >> Thanks, > >> Penghui > >> > >> On Wed, Dec 7, 2022 at 11:46 AM Yunze Xu <[email protected]> > >> wrote: > >> > >>> I agree. It should have required the PIP. > >>> > >>> I have another question. Is there any document to describe these > >>> metrics? I think the metrics body should be documented well to avoid > >>> breaking changes. Some external applications might parse the metrics > >>> according to a specific structure. > >>> > >>> Thanks, > >>> Yunze > >>> > >>> On Wed, Dec 7, 2022 at 11:38 AM PengHui Li <[email protected]> wrote: > >>>> > >>>> Hi all, > >>>> > >>>> I would like to start a discussion about requiring a proposal for > Admin > >>>> API/CLI > >>>> and metrics changes. > >>>> > >>>> Here are some recent examples that changed the Admin API but without > >>>> proposals. > >>>> I just checked the commit logs. Maybe some have a proposal. Just > forgot > >>> to > >>>> add > >>>> the proposal link to the PR. > >>>> > >>>> https://github.com/apache/pulsar/pull/18218 > >>>> https://github.com/apache/pulsar/pull/17153 > >>>> https://github.com/apache/pulsar/pull/16167 > >>>> https://github.com/apache/pulsar/pull/14930 > >>>> https://github.com/apache/pulsar/pull/17337 > >>>> > >>>> And here are metrics-related proposals. But looks like we don't have a > >>>> clear rule > >>>> for this part (the proposal is required or not) > >>>> > >>>> https://github.com/apache/pulsar/issues/18319 > >>>> https://github.com/apache/pulsar/issues/18560 > >>>> > >>>> As more and more users are using Pulsar in production. > >>>> But the Admin API changes and metrics changes have > >>>> not required a proposal. This may pose a risk to users. > >>>> The proposal will have better visibility, and voting is required. > >>>> > >>>> And actually, all the public API changes are proposals required. > >>>> > >>> > https://github.com/apache/pulsar/blob/master/wiki/proposals/PIP.md#when-is-a-pip-required > >>>> But in fact, this is not strictly enforced. > >>>> > >>>> Is it time to require a proposal for Admin API/CLI and metrics > changes? > >>>> > >>>> Thanks, > >>>> Penghui > >>> > >
