osdir.com


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: calcite git commit: tests: add TestUtilTest to CalciteSuite


Julian>Please adhere to the convention for commit comments: start with a
capital letter. I would only use a component prefix (“tests:”) if it
clarifies.

Could you please clarify where the convention is listed?

https://calcite.apache.org/develop/#contributing does not clarify "capital
letter", neither it clarifies "valid component prefixes".
Apparently the convention is to use [CALCITE- prefix, however I have not
created JIRA issue and there's no GitHub PR for this commit and alike (e.g.
6496cb76301e7191 "test: add testSqlAdvisorTableInSchema", 7088dc7261d2
"SqlTestFactory: use lazy initialization of objects", etc).
That was intentional since I am sure it is just an extra work with no real
pay-off for such trivial changes.

The intention for "test:" was to clarify that the commit does not change
production code, so everyone can ignore it.

Do you mean "Add TestUtilTest to CalciteSuite" is way better commit
message? I doubt so since this variation of the message would require one
to parse the message in order to understand that it is test-only commit (it
does not fix bugs, it does not add features, it just updates tests).

In other words, "tests: " (well, it should have been "test: ", but anyway)
does summarize the nature of the commit in a single word. I'm sure that
clarifies a lot, so I use that.

Could you please look a the following commit messages once again? Do you
still think there's a better way to write them?
If so, I'm all ears, no kidding.

tests: add TestUtilTest to CalciteSuite
test: update test name current -> javaMajorVersionExceeds6
test: add testSqlAdvisorTableInSchema

Vladimir