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>
> > ------------------------------
> > ------------------------------
> >

Reply via email to