You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by vl...@apache.org on 2018/08/09 19:19:41 UTC
calcite git commit: tests: add TestUtilTest to CalciteSuite
Repository: calcite
Updated Branches:
refs/heads/master 0e6733bf8 -> 3c6b5ec75
tests: add TestUtilTest to CalciteSuite
Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/3c6b5ec7
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/3c6b5ec7
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/3c6b5ec7
Branch: refs/heads/master
Commit: 3c6b5ec759caadabb67f09d7a4963cc7d9386d0c
Parents: 0e6733b
Author: Vladimir Sitnikov <si...@gmail.com>
Authored: Thu Aug 9 22:19:27 2018 +0300
Committer: Vladimir Sitnikov <si...@gmail.com>
Committed: Thu Aug 9 22:19:27 2018 +0300
----------------------------------------------------------------------
core/src/test/java/org/apache/calcite/test/CalciteSuite.java | 2 ++
1 file changed, 2 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/calcite/blob/3c6b5ec7/core/src/test/java/org/apache/calcite/test/CalciteSuite.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/CalciteSuite.java b/core/src/test/java/org/apache/calcite/test/CalciteSuite.java
index d87efb0..7042010 100644
--- a/core/src/test/java/org/apache/calcite/test/CalciteSuite.java
+++ b/core/src/test/java/org/apache/calcite/test/CalciteSuite.java
@@ -60,6 +60,7 @@ import org.apache.calcite.util.PermutationTestCase;
import org.apache.calcite.util.PrecedenceClimbingParserTest;
import org.apache.calcite.util.ReflectVisitorTest;
import org.apache.calcite.util.SourceTest;
+import org.apache.calcite.util.TestUtilTest;
import org.apache.calcite.util.UtilTest;
import org.apache.calcite.util.graph.DirectedGraphTest;
import org.apache.calcite.util.mapping.MappingTest;
@@ -99,6 +100,7 @@ import org.junit.runners.Suite;
SqlValidatorFeatureTest.class,
VolcanoPlannerTraitTest.class,
InterpreterTest.class,
+ TestUtilTest.class,
VolcanoPlannerTest.class,
HepPlannerTest.class,
TraitPropagationTest.class,
Re: calcite git commit: tests: add TestUtilTest to CalciteSuite
Posted by Julian Hyde <jh...@apache.org>.
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 <si...@gmail.com> 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
Re: calcite git commit: tests: add TestUtilTest to CalciteSuite
Posted by Vladimir Sitnikov <si...@gmail.com>.
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
Re: calcite git commit: tests: add TestUtilTest to CalciteSuite
Posted by Julian Hyde <jh...@apache.org>.
Vladimir,
Please adhere to the convention for commit comments: start with a capital letter. I would only use a component prefix (“tests:”) if it clarifies.
Julian
> On Aug 9, 2018, at 12:19 PM, vladimirsitnikov@apache.org wrote:
>
> Repository: calcite
> Updated Branches:
> refs/heads/master 0e6733bf8 -> 3c6b5ec75
>
>
> tests: add TestUtilTest to CalciteSuite
>
>
> Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
> Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/3c6b5ec7
> Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/3c6b5ec7
> Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/3c6b5ec7
>
> Branch: refs/heads/master
> Commit: 3c6b5ec759caadabb67f09d7a4963cc7d9386d0c
> Parents: 0e6733b
> Author: Vladimir Sitnikov <si...@gmail.com>
> Authored: Thu Aug 9 22:19:27 2018 +0300
> Committer: Vladimir Sitnikov <si...@gmail.com>
> Committed: Thu Aug 9 22:19:27 2018 +0300
>
> ----------------------------------------------------------------------
> core/src/test/java/org/apache/calcite/test/CalciteSuite.java | 2 ++
> 1 file changed, 2 insertions(+)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/calcite/blob/3c6b5ec7/core/src/test/java/org/apache/calcite/test/CalciteSuite.java
> ----------------------------------------------------------------------
> diff --git a/core/src/test/java/org/apache/calcite/test/CalciteSuite.java b/core/src/test/java/org/apache/calcite/test/CalciteSuite.java
> index d87efb0..7042010 100644
> --- a/core/src/test/java/org/apache/calcite/test/CalciteSuite.java
> +++ b/core/src/test/java/org/apache/calcite/test/CalciteSuite.java
> @@ -60,6 +60,7 @@ import org.apache.calcite.util.PermutationTestCase;
> import org.apache.calcite.util.PrecedenceClimbingParserTest;
> import org.apache.calcite.util.ReflectVisitorTest;
> import org.apache.calcite.util.SourceTest;
> +import org.apache.calcite.util.TestUtilTest;
> import org.apache.calcite.util.UtilTest;
> import org.apache.calcite.util.graph.DirectedGraphTest;
> import org.apache.calcite.util.mapping.MappingTest;
> @@ -99,6 +100,7 @@ import org.junit.runners.Suite;
> SqlValidatorFeatureTest.class,
> VolcanoPlannerTraitTest.class,
> InterpreterTest.class,
> + TestUtilTest.class,
> VolcanoPlannerTest.class,
> HepPlannerTest.class,
> TraitPropagationTest.class,
>