osdir.com


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

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


I’ve sent one or two emails on the subject in the last few months.

But mostly it’s a convention, "a way in which something is usually done”; the rule is to follow the same style as other commits.

Julian


> On Aug 9, 2018, at 1:42 PM, Vladimir Sitnikov <sitnikov.vladimir@xxxxxxxxx> wrote:
> 
> 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