Hi team, This is a continued thread from the last one.
Feel free to vote and comment on this issue, thank you! ~~~~~~~~~~~~~~~~ Vote: For PR titles, which convention should we follow? - Angular convention [1] - Our customized convention (it's customized based on Angular) [2] ~~~~~~~~~~~~~~~~ The differences between Angular and ours are: 1. Definition 1.1 Property - Angular: [type] is required, [scope] is optional - Ours: [type] and [scope] are required 1.2 Content - Angular: ci, test, and docs belong to [type] - Ours: ci, test, and docs belong to [scope] because I think [type] defines "what action do you make" (eg. add/delete/update/...), while [scope] defines "where do you make changes". 2. Punctuation - Angular: parenthesis and exclaim points are used - Ours: brackets are used ~~~~~~~~~~~~~~~~ Comparison examples Taking existing Pulsar PR titles as examples: Example 1 - Angular: fix: Filter out already deleted entries again before sending messages to consumers - Ours: [fix][broker] Filter out already deleted entries again before sending messages to consumers Example 2 - Angular: ci: add flaky test issues and PRs to flaky test project - Ours: [feat][ci] Add flaky test issues and PRs to flaky test project Example 3 - Angular: docs: Document configuration added by PIP-145 doc - Ours: [improve][doc] Document configuration added by PIP-145 doc ~~~~~~~~~~~~~~~~ I prefer our customized convention because: - It makes PR titles more clear and self-explanatory. - No need to change users' habits since many people in the community have followed and gotten used to it for several months [3]. ~~~~~~~~~~~~~~~~ Thoughts? Thank you! [1] https://github.com/angular/angular/blob/main/CONTRIBUTING.md#commit-message-header [2] https://docs.google.com/document/d/1d8Pw6ZbWk-_pCKdOmdvx9rnhPiyuxwq60_TrD68d7BA/edit?pli=1#bookmark=id.y8943h392zno [3] https://github.com/apache/pulsar/pulls On Wed, Aug 10, 2022 at 6:11 PM Yu <li...@apache.org> wrote: > > Hi team, > > Thanks for all your suggestions! > > I'll move PIP 198 forward since we get 3 binding votes (Penghui, Lari, > Michael). > > For the implementation details, I've got many different suggestions, which > can be summarized into 2 major issues: > > - Issue 1: which convention should we follow? > Angular [1] or our existing one (customized based on Angular) [2]? > > - Issue 2: how to define [type] and [scope]? > For example, abbreviations. > > To avoid chaos and personal bias, I'll initiate polls one by one and let > the community decide. > > Once we reach a consensus on issue 1, we can move forward to issue 2. > > [1] > https://github.com/angular/angular/blob/main/CONTRIBUTING.md#commit-message-header > [2] > https://docs.google.com/document/d/1d8Pw6ZbWk-_pCKdOmdvx9rnhPiyuxwq60_TrD68d7BA/edit?pli=1#bookmark=id.y8943h392zno > > Yu > > On Wed, Aug 10, 2022 at 6:05 PM Yu <li...@apache.org> wrote: > >> Hi Lari, >> >> Thanks for your suggestions! Please see my replies inline. >> >> > Would it be possible to improve the proposal in a way that the valid >> prefixes for type and component are in a file in the repository and the >> possible checker would use this file as the source of truth? >> >> Yes, it's the same as we did previously [1]. >> >> > I hope we could get rid of the brackets too and simply use a similar >> format as Angular does. >> >> I agree that the PR title should be as much concise as possible, but I >> prefer our customized convention because: >> >> (1) Compared with our current convention (customized based on Angular) >> [2], seems that Angular only saves one char. >> >> For example, >> >> - Angular: feat(broker): add xxx → (): occupy 3 chars >> - Ours: [feat][broker] add xxx → (): occupy 4 chars >> >> (2) I agree with Michael. Brackets make info clearer. >> >> Yu >> >> [1] >> https://github.com/apache/pulsar/pull/16836/files#diff-30f66e07c98171a7ed7bd1f1f873a2dbfb05da069ec859af82dd6bc05048c2c5 >> [2] >> https://docs.google.com/document/d/1d8Pw6ZbWk-_pCKdOmdvx9rnhPiyuxwq60_TrD68d7BA/edit?pli=1#bookmark=id.y8943h392zno >> >> On Tue, Aug 9, 2022 at 5:47 PM Lari Hotari <lhot...@apache.org> wrote: >> >>> +1, with some conditions about the details of PIP 198 that are listed >>> below: >>> >>> Would it be possible to improve the proposal in a way that the valid >>> prefixes for type and component are in a file in the repository and the >>> possible checker would use this file as the source of truth? Tison already >>> pointed out in a Slack discussion that such a GHA exists which uses a yaml >>> file. >>> >>> I also hope that the prefixes are as short as possible since there's a >>> general recommendation to keep a commit title under 50 characters as I have >>> explained in >>> https://lists.apache.org/thread/67fqbo25oq75wrpsq5s4xw9rr55mlbms . I >>> know it's not a hard limit, but it does harm readability of the commit log >>> in many tools if prefixes use up a majority of the title length. >>> >>> So as long as the prefixes are short and easy, I'm fine with this >>> proposal. >>> >>> I would have hoped that the proposal would have been more like the >>> Angular commit message format, >>> https://github.com/angular/angular/blob/main/CONTRIBUTING.md#-commit-message-format >>> . I like the short prefixes and how the "component" is "scope" and maps to >>> a npm module. >>> >>> In our case, the scope (we are calling this "component") could map >>> directly to the Maven artifactId by droping the "pulsar-" prefix. That >>> would prevent making up new names for the components that are different >>> from existing names. >>> for example: artifactId: pulsar-broker, use "broker" >>> artifactId: pulsar-io-kafke, use "io-kafka" >>> There would be some exceptions in the apache/pulsar repository for the >>> cpp client. That could be client-cpp (from directory name >>> "pulsar-client-cpp" by dropping the "pulsar-" prefix). >>> >>> Another concern that I had was about duplicating the information with >>> labels. Tison explained to my that the automation could add the labels >>> based on the title and the user wouldn't have to add duplicate information >>> if such a solution exists. >>> >>> Summary: >>> I hope that PIP 198 could be revisited with the proposed way to map the >>> component name directly from the Maven artifactId. Another request is to >>> shorten the type (use "feat" instead of "feature", etc.) to save >>> characters. >>> I hope we could get rid of the brackets too and simply use a similar >>> format as Angular does. >>> >>> -Lari >>> >>> On 2022/08/04 08:12:21 Yu wrote: >>> > Hi team, >>> > >>> > It has been 4 months since we discussed the [Guideline] Pulsar PR >>> Naming >>> > Convention [1]. >>> > >>> > Nowadays, when reading the PR list [2], you’ll find more and more >>> people >>> > follow and get used to this rule. >>> > >>> > It improves collaboration efficiency, that is great! >>> > >>> > This makes us think about moving the rule forward, that is, >>> standardizing >>> > PR title naming using GitHub Actions, which is a more efficient way. >>> > >>> > So we'd like to start a vote on PIP 198: Standardize PR Naming >>> Convention >>> > using GitHub Actions [3]. >>> > >>> > >>> > This proposal contains: >>> > >>> > - Why do this? >>> > >>> > - How do this? >>> > >>> > - Pre-discussions and other thoughts >>> > >>> > Feel free to comment, thank you! >>> > >>> > [1] https://lists.apache.org/thread/sk9ops3t94jmzc5tndk08y9khf7pj6so >>> > >>> > [2] https://github.com/apache/pulsar/pulls >>> > >>> > [3] >>> > >>> https://docs.google.com/document/d/1sJlUNAHnYAbvu9UtEgCrn_oVTnVc1M5nHC19x1bFab4/edit?pli=1# >>> > >>> > >>> > Yu, Max, mangoGoForward >>> > >>> >>
