git.net

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

Re: calcite-avatica git commit: [CALCITE-2412] Add appveyor.yml to have tests on Windows against jdk1.8, jdk9, jdk10 Add Appveyor badge Add -DskipDockerCheck because of CALCITE-2385 and to make it sync with travis.yml


Will a shell script (instead of bringing in maven) running a regex check (our use-case seems simple enough for a regex check so far) be sufficient? This would avoid the need to set up a Java + maven environment to commit code (in my case, I prefer to run maven and Java in docker containers).

Francis

On 20/11/2018 8:00 am, Michael Mior wrote:
Sorry, that link should have been
https://github.com/phillipuniverse/githook-maven-plugin. Anyway, I don't
have experience with any particular plugin, but git hooks seem to be the
obvious way to go and that's the first one I found.

--
Michael Mior
mmior@xxxxxxxxxx


Le lun. 19 nov. 2018 à 15:36, Enrico Olivelli <eolivelli@xxxxxxxxx> a
écrit :

Il lun 19 nov 2018, 21:29 Michael Mior <mmior@xxxxxxxxxx> ha scritto:

How about making use of
https://github.com/olukyrich/githook-maven-plugin?
A post-commit hooks in git seems to be an easy way to achieve this.
Unfortunately, it would require that each fresh clone of the repository
has
a one-time command run to install the hook.

Michael,
It seems this plugin is notnot available on maven central
https://github.com/olukyrich/githook-maven-plugin

Do you have experience with it?
Enrico

--
Michael Mior
mmior@xxxxxxxxxx


Le lun. 19 nov. 2018 à 14:49, Julian Hyde <jhyde@xxxxxxxxxx> a écrit :

On Nov 19, 2018, at 11:19 AM, Vladimir Sitnikov <
sitnikov.vladimir@xxxxxxxxx> wrote:
Well, a rule of "first line should be separated by a blank line"
seems
to
be automatable.
The rule of "CALCITE-XXX should be in [...]" seems to be automatable.
And so on.
Yes, we should do that.

However there are things that automation could never achieve, so let’s
continue to talk about those, also.

setDynamicParam did not look good enough to me

https://git-wip-us.apache.org/repos/asf?p=calcite.git;a=blobdiff;f=core/src/main/java/org/apache/calcite/runtime/ResultSetEnumerable.java;h=52c11f9aaf752cea367ae921c04a20e5e6da5488;hp=771772f1ea39d74eee6e9f889ae8d6500f129c7f;hb=53e15af6c5e8e782b2edcd7f5bf4f5f32225d110;hpb=02ca9bc995cac5b4b97855a4d06df46e632d7c22
I'm inclined to incline Avatica to expose
TypedValue.setToPreparedStatement(PreparedStatement ps, int index)
kind
of
API, so ResultSetEnumerable.setDynamicParam could be removed
altogether.
I agree. The commit didn’t seem quite perfect to me either. However, it
seemed to be progress. Log an Avatica JIRA if you have ideas for how to
improve it further. Since it will be in Avatica it will take a while to
bubble through the release cycle.

Julian


--


-- Enrico Olivelli