I have merged https://github.com/apache/calcite/commit/3c19347cc45349a21a8c97d6f6e8d3e9f596070f with a new lint rule. I had to force-push to amended the previous two commits because the linter checks the N most recent commits.
Julian > On Jan 4, 2024, at 11:28 AM, Julian Hyde <jhyde.apa...@gmail.com> wrote: > > For the record, I think that ‘Lint’ and ’Typo’ are perfectly good commit > messages. Simple changes should look simple. > > We have already documented the standards: > https://calcite.apache.org/develop/#contributing. > > > >> On Jan 3, 2024, at 7:07 PM, Forward Xu <forwardxu...@gmail.com> wrote: >> >> If the fix is very small and there is no related work order or issue, we >> can briefly describe the problem and explain the reason for the fix when >> submitting the PR. If we need to standardize this type of PR submission, we >> can form some PR templates or documentation. >> >> Best, >> ForwardXu >> >> Julian Hyde <jh...@apache.org> 于2024年1月4日周四 04:21写道： >> >>> It's so difficult to agree on a vocabulary that everyone will agree >>> on, I don't know whether we should even try. >>> >>> I follow the universal rule: "When in Rome, do as the Romans do". >>> Which is to say, follow the existing standard (even if it's >>> undocumented, which it probably is) and don't invent your own. If I'm >>> about to make a commit that fixes a typo, or fixes lint errors, or >>> upgrades a component, I browse the commit log to see what commit >>> messages people have used for similar changes. >>> >>> Julian >>> >>> On Wed, Jan 3, 2024 at 4:11 AM Benchao Li <libenc...@apache.org> wrote: >>>> >>>> The word [MINOR] is very coarse-grained to me, as Stamatis mentions >>>> above, it may contains small improvements to javadoc, site, test, >>>> error message, method/variable name, etc. And it is subjective to the >>>> author/committer whether a PR should be minor. >>>> >>>> Looking at the git history, I found it useful when a commit without >>>> Jira id starts with "Site:", "Refactor:", "Code style:", "Lint". >>>> Maybe we can add more useful prefixes for common small changes, such >>>> as "Mailmap: ", "Javadoc: ", "Typo:". For others which are not very >>>> common small changes, just a clear/concise commit message without any >>>> prefix is good enough. >>>> >>>> Anyway, we'd better reach an agreement when we can omit Jira id and >>>> consider a PR is small. >>>> >>>> Stamatis Zampetakis <zabe...@gmail.com> 于2024年1月3日周三 18:27写道： >>>>> >>>>> The presence of the "minor" keyword in the commit summary is a bit >>>>> redundant. I would argue that if the message is precise enough the >>>>> person reading it can infer it is minor or not. >>>>> >>>>> Moreover, the minor classification is subjective. Some people consider >>>>> minor things that do not change code at all. Others consider minor >>>>> things that don't touch production code. The addition of tests and >>>>> fixing a broken build is also considered minor by few people. All >>>>> these examples are taken from the current commits which landed in >>>>> Calcite. >>>>> >>>>> For the reasons above, I feel that we don't really need to include >>>>> "minor" in the commit summary but don't feel very strongly about it >>>>> anyways. >>>>> >>>>> In the past we agreed that if a change is trivial/minor we don't need >>>>> a JIRA id. Although, this is convenient for getting this committed >>>>> faster without too much hassle it has some drawbacks. >>>>> >>>>> If at some point in the future we want to attach more information to a >>>>> particular commit, this is not possible since we cannot modify the git >>>>> history. When there is a JIRA we can always add new comments and >>>>> clarifications there even if the ticket is resolved. >>>>> >>>>> Having a JIRA id is also a convenient way and readable way for >>>>> referring to particular commits. The commit hash can be used to >>>>> identify a commit in Apache main branch but if the same commit is >>>>> backported to other forks/branches using the hash becomes more >>>>> cumbersome. This is especially useful in downstream projects and forks >>>>> for tracking that a specific change landed in various branches. >>>>> >>>>> Maybe in the future we should reconsider the optionality of the JIRA >>>>> id in the commit summary. If this happens then [CALCITE-XXXXX][MINOR] >>>>> would start getting overly long so this may be another argument >>>>> against including "minor" in the message. >>>>> >>>>> Best, >>>>> Stamatis >>>>> >>>>> On Wed, Jan 3, 2024 at 10:25 AM Ran Tao <chucheng...@gmail.com> wrote: >>>>>> >>>>>> This format most likely comes from other open source projects. >>>>>> If calcite has its own specifications, such as how to set the title >>> for PRs >>>>>> that do not require a jira name, >>>>>> IMHO, it can be introduced in the contribution doc. >>>>>> Commiters can also review PRs according to this specification. >>>>>> >>>>>> Best Regards, >>>>>> Ran Tao >>>>>> >>>>>> >>>>>> Istvan Toth <st...@cloudera.com.invalid> 于2024年1月3日周三 16:11写道： >>>>>> >>>>>>> Perhaps the square bracket convention ? >>>>>>> If the ticket starts with CALICITE-\d+ , then make sure that the >>> JIRA >>>>>>> ticket id is between brackets. >>>>>>> >>>>>>> Also check for Gerrit Change IDs which are often added >>> automatically, and a >>>>>>> paint to remove. >>>>>>> >>>>>>> Istvan >>>>>>> >>>>>>> On Tue, Jan 2, 2024 at 10:50 PM Tanner Clary < >>> tannercl...@google.com >>>>>>> .invalid> >>>>>>> wrote: >>>>>>> >>>>>>>> I like the [MINOR] prefix because it makes it easy to identify >>> simple >>>>>>>> commits (via grep or ctrl+f), the same way [CALCITE-1234] makes >>> it easy >>>>>>> to >>>>>>>> find commits related to [CALCITE-1234]. I also like that it >>> maintains the >>>>>>>> "[...]" styling at the beginning of the commit message. >>>>>>>> >>>>>>>> Neither of these reasons is strong enough for me to say I >>> oppose, just >>>>>>> some >>>>>>>> minor (heh) counter-arguments. >>>>>>>> >>>>>>>> -Tanner >>>>>>>> >>>>>>>> On Tue, Jan 2, 2024 at 1:05 PM Julian Hyde <jh...@apache.org> >>> wrote: >>>>>>>> >>>>>>>>> Ralph Waldo Emerson once wrote: “A foolish consistency is the >>>>>>>>> hobgoblin of little minds, adored by little statesmen and >>> philosophers >>>>>>>>> and divines." >>>>>>>>> >>>>>>>>> That said, people tend to bring conventions from other >>> projects to >>>>>>>>> Calcite, and we end up with chaos. By which I mean, lots of >>>>>>>>> self-expression, but no standards, and therefore commit >>> messages that >>>>>>>>> have lower information content, and more work for the release >>> manager >>>>>>>>> coercing them into a consistent change log. >>>>>>>>> >>>>>>>>> In Calcite we have not used '[MINOR]' as a prefix to minor >>> commits. If >>>>>>>>> it is minor, it doesn't need a jira case, and doesn't need a >>> prefix. >>>>>>>>> But a few commits with [MINOR] crept in, starting about a year >>> ago. >>>>>>>>> Once or twice, I asked people to remove them, but the PRs had >>> already >>>>>>>>> been merged. >>>>>>>>> >>>>>>>>> Any objections if I add a lint rule to fail the build if the >>> commit >>>>>>>>> message contains [MINOR]? >>>>>>>>> >>>>>>>>> While I'm there, any other standards we should enforce? >>>>>>>>> >>>>>>>>> Julian >>>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> *István Tóth* | Sr. Staff Software Engineer >>>>>>> *Email*: st...@cloudera.com >>>>>>> cloudera.com <https://www.cloudera.com> >>>>>>> [image: Cloudera] <https://www.cloudera.com/> >>>>>>> [image: Cloudera on Twitter] <https://twitter.com/cloudera> >>> [image: >>>>>>> Cloudera on Facebook] <https://www.facebook.com/cloudera> [image: >>> Cloudera >>>>>>> on LinkedIn] <https://www.linkedin.com/company/cloudera> >>>>>>> ------------------------------ >>>>>>> ------------------------------ >>>>>>> >>>> >>>> >>>> >>>> -- >>>> >>>> Best, >>>> Benchao Li >>> >
