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 <[email protected]> 于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 <[email protected]> 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 <[email protected]> 于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 <[email protected]> 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 <[email protected]> 于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 < > [email protected] > > > > > .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 <[email protected]> > 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*: [email protected] > > > > > 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 >
