You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Julian Hyde <jh...@apache.org> on 2018/11/19 18:34:32 UTC
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
Sergey,
Can you please force-push to clean up the commit message. You didn’t leave a space between the first and second lines, so git created a multi-line summary. We need a simple, descriptive one-liner (ideally describing everything in the change, albeit at a high level).
Sorry to harp on to everyone about commit messages. But a clear code history is essential for future contributors, and clear release notes are essential for our users.
Julian
> On Nov 19, 2018, at 5:38 AM, snuyanzin@apache.org wrote:
>
> Repository: calcite-avatica
> Updated Branches:
> refs/heads/master 9199fef02 -> 12eea22f1
>
>
> [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
>
> Close apache/calcite-avatica#65
>
>
> Project: http://git-wip-us.apache.org/repos/asf/calcite-avatica/repo
> Commit: http://git-wip-us.apache.org/repos/asf/calcite-avatica/commit/12eea22f
> Tree: http://git-wip-us.apache.org/repos/asf/calcite-avatica/tree/12eea22f
> Diff: http://git-wip-us.apache.org/repos/asf/calcite-avatica/diff/12eea22f
>
> Branch: refs/heads/master
> Commit: 12eea22f101780a68325a1595774e269616b4ec0
> Parents: 9199fef
> Author: snuyanzin <sn...@gmail.com>
> Authored: Wed Jul 11 10:26:01 2018 +0300
> Committer: snuyanzin <sn...@gmail.com>
> Committed: Mon Nov 19 16:33:57 2018 +0300
>
> ----------------------------------------------------------------------
> README.md | 1 +
> appveyor.yml | 43 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 44 insertions(+)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/calcite-avatica/blob/12eea22f/README.md
> ----------------------------------------------------------------------
> diff --git a/README.md b/README.md
> index f5c84e7..ffd1ab0 100644
> --- a/README.md
> +++ b/README.md
> @@ -17,6 +17,7 @@ limitations under the License.
> {% endcomment %}
> -->
> [](https://travis-ci.org/apache/calcite-avatica)
> +[](https://ci.appveyor.com/project/ApacheSoftwareFoundation/calcite-avatica)
>
> # Apache Calcite -- Avatica
>
>
> http://git-wip-us.apache.org/repos/asf/calcite-avatica/blob/12eea22f/appveyor.yml
> ----------------------------------------------------------------------
> diff --git a/appveyor.yml b/appveyor.yml
> new file mode 100644
> index 0000000..cd397a2
> --- /dev/null
> +++ b/appveyor.yml
> @@ -0,0 +1,43 @@
> +# Configuration file for Appveyor continuous integration.
> +#
> +# Licensed to the Apache Software Foundation (ASF) under one or more
> +# contributor license agreements. See the NOTICE file distributed with
> +# this work for additional information regarding copyright ownership.
> +# The ASF licenses this file to you under the Apache License, Version 2.0
> +# (the "License"); you may not use this file except in compliance with
> +# the License. You may obtain a copy of the License at
> +#
> +# http://www.apache.org/licenses/LICENSE-2.0
> +#
> +# Unless required by applicable law or agreed to in writing, software
> +# distributed under the License is distributed on an "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> +# See the License for the specific language governing permissions and
> +# limitations under the License.
> +#
> +version: '{build}'
> +image: Visual Studio 2017
> +clone_depth: 10000
> +# branches to build
> +branches:
> + # whitelist
> + only:
> + - master
> + - new-master
> + - javadoc
> + - /^branch-.*$/
> + - /^[0-9]+-.*$/
> +matrix:
> + fast_finish: true
> +environment:
> + matrix:
> + - JAVA_HOME: C:\Program Files\Java\jdk1.8.0
> + - JAVA_HOME: C:\Program Files\Java\jdk9
> + - JAVA_HOME: C:\Program Files\Java\jdk10
> +build_script:
> + - mvn clean -V install -DskipTests -Dmaven.javadoc.skip=true -Djavax.net.ssl.trustStorePassword=changeit -DskipDockerCheck
> +test_script:
> + - mvn -Dsurefire.useFile=false -Djavax.net.ssl.trustStorePassword=changeit verify javadoc:javadoc javadoc:test-javadoc -DskipDockerCheck
> +cache:
> + - C:\maven\
> + - C:\Users\appveyor\.m2
>
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
Posted by Sergey Nuyanzin <sn...@gmail.com>.
Julian, thank you for your feedback.
I changed the commit message to one-liner, will keep it in mind next time
On Mon, Nov 19, 2018 at 10:09 PM Julian Hyde <jh...@apache.org> wrote:
> I don’t think it’s possible to automate. Crafting good messages is an art,
> not a science. Which is not to say that developers can’t get good at it,
> with a little practice.
>
> I’m sorry if you feel intimidated by my critiques. The fact that people
> can criticize commits after the event is a good thing - it allows us to
> move faster, because PRs do not have to be made 100% perfect before they
> are submitted. I try to make my criticism as gently as possible, and bear
> in mind that everyone has this project’s best interests in mind.
>
> Regarding
> https://git-wip-us.apache.org/repos/asf?p=calcite.git;a=commit;h=53e15af6c5e8e782b2edcd7f5bf4f5f32225d110.
> I wish that you had committed that change, and I was a bit surprised that
> you, as a committer, had not already committed it. It would have saved me 2
> hours over the weekend reviewing and revising it. I included both
> contributors’ names in the message because I had squashed together two
> commits from different authors. I don’t know whether I broke guidelines,
> but I was acting in good faith.
>
> Julian
>
>
> > On Nov 19, 2018, at 10:55 AM, Vladimir Sitnikov <
> sitnikov.vladimir@gmail.com> wrote:
> >
> >> Sorry to harp on to everyone about commit messages. But a clear code
> > history is essential for future contributors, and clear release notes are
> > essential for our users.
> >
> > Sorry for highjacking the thread, however it would be really great if the
> > verification could be automated.
> > The fear of "Julian claiming the commit message not being good enough" is
> > very real for me, and it is part of the reason I asked Michael to commit
> > "[CALCITE-2266] Implement SQL:2016 JSON functions" (and he was kind
> enough
> > :) ! )
> >
> > PS. I'm not sure if
> >
> https://git-wip-us.apache.org/repos/asf?p=calcite.git;a=commit;h=53e15af6c5e8e782b2edcd7f5bf4f5f32225d110
> > is
> > in line with "standards".
> > Should committers (e.g. "Vladimir Sitnikov") be placed to the commit
> > headline (first line)?
> > I thought we don't place committer names to the headline, however the Art
> > of Calcite Commit Message is close to black magic to me.
> >
> > Vladimir
>
>
--
Best regards,
Sergey
Re: Automatic validation for commit messages
Posted by Francis Chuang <fr...@apache.org>.
Re: Vladimir's suggestion to run the hooks only on master. I think this
is a good idea. Sometimes I like to add a few test commits on my branch
to test
whether something works correctly (i.e. testing avatica release goal)
before making a proper commit.
On 20/11/2018 6:24 pm, Julian Hyde wrote:
> Before we embark on this, are we all agreed that once this hook is in place, no one will say “Hey, why are you complaining about my commit message? It passed the validation.”
>
> People seem to think that any formatting errors not found by checkstyle are - by definition - not formatting errors, and I don’t want to repeat the same mistake here.
>
> Julian
>
>
>> On Nov 19, 2018, at 11:13 PM, Vladimir Sitnikov <si...@gmail.com> wrote:
>>
>> I've updated the subject to match the actual content
>>
>> Francis>I feel that a git hook would probably still be the easiest and
>> simplest
>> Francis>way to get started
>>
>> 1) Just in case: I guess we need to confine automatic validation for
>> "master" branch only (or alike).
>>
>> For instance, I tend to create tons of oops/fixup/t/tmp commits, and I
>> don't really want for the validator to blame me on each and every commit.
>> Ideally, it would validate only for "on push to apache.../master" (and/or
>> on commit to "local master branch")
>>
>> 2) Starting from Git 2.9, hook location could be configured via
>> core.hooksPath (see https://git-scm.com/docs/githooks ), so I guess we
>> would be fine if we just add the hooks to .githooks folder + a command to
>> activate it.
>> It's not clear if we should try to activate the hooks automatically (e.g.
>> at mvn validate or something like that), however we could do that for
>> Travis and it could produce decent coverage.
>>
>> PS. They say, Git on Windows executes hooks via its own bash, so the hooks
>> should just work: https://stackoverflow.com/a/18278072/1261287
>>
>> Vladimir
Re: Automatic validation for commit messages
Posted by Francis Chuang <fr...@apache.org>.
On 20/11/2018 6:24 pm, Julian Hyde wrote:
> Before we embark on this, are we all agreed that once this hook is in place, no one will say “Hey, why are you complaining about my commit message? It passed the validation.”
I am +1 for this. Validation rules are always going to have holes and
edge cases that cannot be fully covered in the initial implementation.
Re: Automatic validation for commit messages
Posted by Julian Hyde <jh...@apache.org>.
Before we embark on this, are we all agreed that once this hook is in place, no one will say “Hey, why are you complaining about my commit message? It passed the validation.”
People seem to think that any formatting errors not found by checkstyle are - by definition - not formatting errors, and I don’t want to repeat the same mistake here.
Julian
> On Nov 19, 2018, at 11:13 PM, Vladimir Sitnikov <si...@gmail.com> wrote:
>
> I've updated the subject to match the actual content
>
> Francis>I feel that a git hook would probably still be the easiest and
> simplest
> Francis>way to get started
>
> 1) Just in case: I guess we need to confine automatic validation for
> "master" branch only (or alike).
>
> For instance, I tend to create tons of oops/fixup/t/tmp commits, and I
> don't really want for the validator to blame me on each and every commit.
> Ideally, it would validate only for "on push to apache.../master" (and/or
> on commit to "local master branch")
>
> 2) Starting from Git 2.9, hook location could be configured via
> core.hooksPath (see https://git-scm.com/docs/githooks ), so I guess we
> would be fine if we just add the hooks to .githooks folder + a command to
> activate it.
> It's not clear if we should try to activate the hooks automatically (e.g.
> at mvn validate or something like that), however we could do that for
> Travis and it could produce decent coverage.
>
> PS. They say, Git on Windows executes hooks via its own bash, so the hooks
> should just work: https://stackoverflow.com/a/18278072/1261287
>
> Vladimir
Automatic validation for commit messages
Posted by Vladimir Sitnikov <si...@gmail.com>.
I've updated the subject to match the actual content
Francis>I feel that a git hook would probably still be the easiest and
simplest
Francis>way to get started
1) Just in case: I guess we need to confine automatic validation for
"master" branch only (or alike).
For instance, I tend to create tons of oops/fixup/t/tmp commits, and I
don't really want for the validator to blame me on each and every commit.
Ideally, it would validate only for "on push to apache.../master" (and/or
on commit to "local master branch")
2) Starting from Git 2.9, hook location could be configured via
core.hooksPath (see https://git-scm.com/docs/githooks ), so I guess we
would be fine if we just add the hooks to .githooks folder + a command to
activate it.
It's not clear if we should try to activate the hooks automatically (e.g.
at mvn validate or something like that), however we could do that for
Travis and it could produce decent coverage.
PS. They say, Git on Windows executes hooks via its own bash, so the hooks
should just work: https://stackoverflow.com/a/18278072/1261287
Vladimir
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
Posted by Francis Chuang <fr...@apache.org>.
The probot plugin looks great. However, we also need something that
works for Jenkins.
I feel that a git hook would probably still be the easiest and simplest
way to get started: we can block bad commit messages before they are
committed.
Francis
On 20/11/2018 8:46 am, Enrico Olivelli wrote:
> Il giorno lun 19 nov 2018 alle ore 22:41 Enrico Olivelli
> <eo...@gmail.com> ha scritto:
>> We can run the script in Travis, so that we will refuse any commit
>> message not compliant with the "specs"
>> In Travis you can run a shell script with uses directly the 'git' command
> What about this github plugin ?
> https://github.com/probot/semantic-pull-requests
>
> Enrico
>
>>
>> Enrico
>>
>> Il giorno lun 19 nov 2018 alle ore 22:35 Francis Chuang
>> <fr...@apache.org> ha scritto:
>>> I am not sure about server-side hooks on ASF's git. It would require
>>> execution privileges on ASF's git instance (for Github, this is not
>>> allowed).
>>>
>>> For Windows, I use WSL (Windows Subsystem for Linux). This is a copy of
>>> Ubuntu or some other supported Linux distro that runs within Windows and
>>> allows the execution of Linux executables on Windows. WSL translates
>>> calls to Windows native calls where needed. The downside is that WSL
>>> only works on Windows 10 1607 (released august/july 2016) and later, so
>>> people using Windows 8 and Windows 7 will not be able to use it.
>>>
>>> Francis
>>>
>>> On 20/11/2018 8:21 am, Julian Hyde wrote:
>>>> Are we allowed to create hooks in ASF’s git?
>>>>
>>>> If we add a shell script, are we excluding Windows folks? (Or can we require them to install Cygwin?)
>>>>
>>>>> On Nov 19, 2018, at 1:15 PM, Michael Mior <mm...@apache.org> wrote:
>>>>>
>>>>> Sure, a shell script would work just fine as well. Really we just need to
>>>>> create a symlink in the .git/hooks directory that points to a script which
>>>>> performs whatever checks we want.
>>>>> --
>>>>> Michael Mior
>>>>> mmior@apache.org
>>>>>
>>>>>
>>>>> Le lun. 19 nov. 2018 à 16:11, Francis Chuang <fr...@apache.org> a
>>>>> écrit :
>>>>>
>>>>>> 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@apache.org
>>>>>>>
>>>>>>>
>>>>>>> Le lun. 19 nov. 2018 à 15:36, Enrico Olivelli <eo...@gmail.com> a
>>>>>>> écrit :
>>>>>>>
>>>>>>>> Il lun 19 nov 2018, 21:29 Michael Mior <mm...@apache.org> 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@apache.org
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Le lun. 19 nov. 2018 à 14:49, Julian Hyde <jh...@apache.org> a écrit :
>>>>>>>>>
>>>>>>>>>>> On Nov 19, 2018, at 11:19 AM, Vladimir Sitnikov <
>>>>>>>>>> sitnikov.vladimir@gmail.com> 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
>>>>>>>>
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
Posted by Enrico Olivelli <eo...@gmail.com>.
Il giorno lun 19 nov 2018 alle ore 22:41 Enrico Olivelli
<eo...@gmail.com> ha scritto:
>
> We can run the script in Travis, so that we will refuse any commit
> message not compliant with the "specs"
> In Travis you can run a shell script with uses directly the 'git' command
What about this github plugin ?
https://github.com/probot/semantic-pull-requests
Enrico
>
>
> Enrico
>
> Il giorno lun 19 nov 2018 alle ore 22:35 Francis Chuang
> <fr...@apache.org> ha scritto:
> >
> > I am not sure about server-side hooks on ASF's git. It would require
> > execution privileges on ASF's git instance (for Github, this is not
> > allowed).
> >
> > For Windows, I use WSL (Windows Subsystem for Linux). This is a copy of
> > Ubuntu or some other supported Linux distro that runs within Windows and
> > allows the execution of Linux executables on Windows. WSL translates
> > calls to Windows native calls where needed. The downside is that WSL
> > only works on Windows 10 1607 (released august/july 2016) and later, so
> > people using Windows 8 and Windows 7 will not be able to use it.
> >
> > Francis
> >
> > On 20/11/2018 8:21 am, Julian Hyde wrote:
> > > Are we allowed to create hooks in ASF’s git?
> > >
> > > If we add a shell script, are we excluding Windows folks? (Or can we require them to install Cygwin?)
> > >
> > >> On Nov 19, 2018, at 1:15 PM, Michael Mior <mm...@apache.org> wrote:
> > >>
> > >> Sure, a shell script would work just fine as well. Really we just need to
> > >> create a symlink in the .git/hooks directory that points to a script which
> > >> performs whatever checks we want.
> > >> --
> > >> Michael Mior
> > >> mmior@apache.org
> > >>
> > >>
> > >> Le lun. 19 nov. 2018 à 16:11, Francis Chuang <fr...@apache.org> a
> > >> écrit :
> > >>
> > >>> 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@apache.org
> > >>>>
> > >>>>
> > >>>> Le lun. 19 nov. 2018 à 15:36, Enrico Olivelli <eo...@gmail.com> a
> > >>>> écrit :
> > >>>>
> > >>>>> Il lun 19 nov 2018, 21:29 Michael Mior <mm...@apache.org> 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@apache.org
> > >>>>>>
> > >>>>>>
> > >>>>>> Le lun. 19 nov. 2018 à 14:49, Julian Hyde <jh...@apache.org> a écrit :
> > >>>>>>
> > >>>>>>>> On Nov 19, 2018, at 11:19 AM, Vladimir Sitnikov <
> > >>>>>>> sitnikov.vladimir@gmail.com> 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
> > >>>>>
> > >>>
> >
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
Posted by Enrico Olivelli <eo...@gmail.com>.
We can run the script in Travis, so that we will refuse any commit
message not compliant with the "specs"
In Travis you can run a shell script with uses directly the 'git' command
Enrico
Il giorno lun 19 nov 2018 alle ore 22:35 Francis Chuang
<fr...@apache.org> ha scritto:
>
> I am not sure about server-side hooks on ASF's git. It would require
> execution privileges on ASF's git instance (for Github, this is not
> allowed).
>
> For Windows, I use WSL (Windows Subsystem for Linux). This is a copy of
> Ubuntu or some other supported Linux distro that runs within Windows and
> allows the execution of Linux executables on Windows. WSL translates
> calls to Windows native calls where needed. The downside is that WSL
> only works on Windows 10 1607 (released august/july 2016) and later, so
> people using Windows 8 and Windows 7 will not be able to use it.
>
> Francis
>
> On 20/11/2018 8:21 am, Julian Hyde wrote:
> > Are we allowed to create hooks in ASF’s git?
> >
> > If we add a shell script, are we excluding Windows folks? (Or can we require them to install Cygwin?)
> >
> >> On Nov 19, 2018, at 1:15 PM, Michael Mior <mm...@apache.org> wrote:
> >>
> >> Sure, a shell script would work just fine as well. Really we just need to
> >> create a symlink in the .git/hooks directory that points to a script which
> >> performs whatever checks we want.
> >> --
> >> Michael Mior
> >> mmior@apache.org
> >>
> >>
> >> Le lun. 19 nov. 2018 à 16:11, Francis Chuang <fr...@apache.org> a
> >> écrit :
> >>
> >>> 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@apache.org
> >>>>
> >>>>
> >>>> Le lun. 19 nov. 2018 à 15:36, Enrico Olivelli <eo...@gmail.com> a
> >>>> écrit :
> >>>>
> >>>>> Il lun 19 nov 2018, 21:29 Michael Mior <mm...@apache.org> 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@apache.org
> >>>>>>
> >>>>>>
> >>>>>> Le lun. 19 nov. 2018 à 14:49, Julian Hyde <jh...@apache.org> a écrit :
> >>>>>>
> >>>>>>>> On Nov 19, 2018, at 11:19 AM, Vladimir Sitnikov <
> >>>>>>> sitnikov.vladimir@gmail.com> 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
> >>>>>
> >>>
>
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
Posted by Francis Chuang <fr...@apache.org>.
I am not sure about server-side hooks on ASF's git. It would require
execution privileges on ASF's git instance (for Github, this is not
allowed).
For Windows, I use WSL (Windows Subsystem for Linux). This is a copy of
Ubuntu or some other supported Linux distro that runs within Windows and
allows the execution of Linux executables on Windows. WSL translates
calls to Windows native calls where needed. The downside is that WSL
only works on Windows 10 1607 (released august/july 2016) and later, so
people using Windows 8 and Windows 7 will not be able to use it.
Francis
On 20/11/2018 8:21 am, Julian Hyde wrote:
> Are we allowed to create hooks in ASF’s git?
>
> If we add a shell script, are we excluding Windows folks? (Or can we require them to install Cygwin?)
>
>> On Nov 19, 2018, at 1:15 PM, Michael Mior <mm...@apache.org> wrote:
>>
>> Sure, a shell script would work just fine as well. Really we just need to
>> create a symlink in the .git/hooks directory that points to a script which
>> performs whatever checks we want.
>> --
>> Michael Mior
>> mmior@apache.org
>>
>>
>> Le lun. 19 nov. 2018 à 16:11, Francis Chuang <fr...@apache.org> a
>> écrit :
>>
>>> 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@apache.org
>>>>
>>>>
>>>> Le lun. 19 nov. 2018 à 15:36, Enrico Olivelli <eo...@gmail.com> a
>>>> écrit :
>>>>
>>>>> Il lun 19 nov 2018, 21:29 Michael Mior <mm...@apache.org> 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@apache.org
>>>>>>
>>>>>>
>>>>>> Le lun. 19 nov. 2018 à 14:49, Julian Hyde <jh...@apache.org> a écrit :
>>>>>>
>>>>>>>> On Nov 19, 2018, at 11:19 AM, Vladimir Sitnikov <
>>>>>>> sitnikov.vladimir@gmail.com> 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
>>>>>
>>>
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
Posted by Francis Chuang <fr...@apache.org>.
Git for Windows includes a Windows port of bash (git bash), so bash
scripts should work just fine.
We just need to be careful with the commands we use as we need to make
sure they are ones that are also shipped with Git for Windows.
For reference: https://stackoverflow.com/a/18278072/624884
On 22/11/2018 1:01 pm, Michael Mior wrote:
> I could be wrong, but my understanding was that on Windows (as on other
> platforms), hooks are simply scripts that are executed. Given that we can't
> run a bash script on Windows (without making unnecessary assumptions about
> the environment), it seems a second script would be needed.
>
> --
> Michael Mior
> mmior@apache.org
>
>
> Le mer. 21 nov. 2018 à 09:47, Vladimir Sitnikov <si...@gmail.com>
> a écrit :
>
>> Michael>Unfortunately, to work on Windows, I believe we would need to write
>> two
>> Michael>versions of the hook.
>>
>> Why do you think so?
>>
>> Vladimir
>>
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
Posted by Michael Mior <mm...@apache.org>.
I could be wrong, but my understanding was that on Windows (as on other
platforms), hooks are simply scripts that are executed. Given that we can't
run a bash script on Windows (without making unnecessary assumptions about
the environment), it seems a second script would be needed.
--
Michael Mior
mmior@apache.org
Le mer. 21 nov. 2018 à 09:47, Vladimir Sitnikov <si...@gmail.com>
a écrit :
> Michael>Unfortunately, to work on Windows, I believe we would need to write
> two
> Michael>versions of the hook.
>
> Why do you think so?
>
> Vladimir
>
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
Posted by Vladimir Sitnikov <si...@gmail.com>.
Michael>Unfortunately, to work on Windows, I believe we would need to write
two
Michael>versions of the hook.
Why do you think so?
Vladimir
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
Posted by Michael Mior <mm...@apache.org>.
Git hooks run client-side, so there's nothing to do on the server.
Unfortunately, to work on Windows, I believe we would need to write two
versions of the hook.
--
Michael Mior
mmior@apache.org
Le lun. 19 nov. 2018 à 16:21, Julian Hyde <jh...@apache.org> a écrit :
> Are we allowed to create hooks in ASF’s git?
>
> If we add a shell script, are we excluding Windows folks? (Or can we
> require them to install Cygwin?)
>
> > On Nov 19, 2018, at 1:15 PM, Michael Mior <mm...@apache.org> wrote:
> >
> > Sure, a shell script would work just fine as well. Really we just need to
> > create a symlink in the .git/hooks directory that points to a script
> which
> > performs whatever checks we want.
> > --
> > Michael Mior
> > mmior@apache.org
> >
> >
> > Le lun. 19 nov. 2018 à 16:11, Francis Chuang <fr...@apache.org>
> a
> > écrit :
> >
> >> 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@apache.org
> >>>
> >>>
> >>> Le lun. 19 nov. 2018 à 15:36, Enrico Olivelli <eo...@gmail.com> a
> >>> écrit :
> >>>
> >>>> Il lun 19 nov 2018, 21:29 Michael Mior <mm...@apache.org> 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@apache.org
> >>>>>
> >>>>>
> >>>>> Le lun. 19 nov. 2018 à 14:49, Julian Hyde <jh...@apache.org> a
> écrit :
> >>>>>
> >>>>>>> On Nov 19, 2018, at 11:19 AM, Vladimir Sitnikov <
> >>>>>> sitnikov.vladimir@gmail.com> 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
> >>>>
> >>
> >>
>
>
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
Posted by Julian Hyde <jh...@apache.org>.
Are we allowed to create hooks in ASF’s git?
If we add a shell script, are we excluding Windows folks? (Or can we require them to install Cygwin?)
> On Nov 19, 2018, at 1:15 PM, Michael Mior <mm...@apache.org> wrote:
>
> Sure, a shell script would work just fine as well. Really we just need to
> create a symlink in the .git/hooks directory that points to a script which
> performs whatever checks we want.
> --
> Michael Mior
> mmior@apache.org
>
>
> Le lun. 19 nov. 2018 à 16:11, Francis Chuang <fr...@apache.org> a
> écrit :
>
>> 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@apache.org
>>>
>>>
>>> Le lun. 19 nov. 2018 à 15:36, Enrico Olivelli <eo...@gmail.com> a
>>> écrit :
>>>
>>>> Il lun 19 nov 2018, 21:29 Michael Mior <mm...@apache.org> 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@apache.org
>>>>>
>>>>>
>>>>> Le lun. 19 nov. 2018 à 14:49, Julian Hyde <jh...@apache.org> a écrit :
>>>>>
>>>>>>> On Nov 19, 2018, at 11:19 AM, Vladimir Sitnikov <
>>>>>> sitnikov.vladimir@gmail.com> 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
>>>>
>>
>>
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
Posted by Michael Mior <mm...@apache.org>.
Sure, a shell script would work just fine as well. Really we just need to
create a symlink in the .git/hooks directory that points to a script which
performs whatever checks we want.
--
Michael Mior
mmior@apache.org
Le lun. 19 nov. 2018 à 16:11, Francis Chuang <fr...@apache.org> a
écrit :
> 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@apache.org
> >
> >
> > Le lun. 19 nov. 2018 à 15:36, Enrico Olivelli <eo...@gmail.com> a
> > écrit :
> >
> >> Il lun 19 nov 2018, 21:29 Michael Mior <mm...@apache.org> 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@apache.org
> >>>
> >>>
> >>> Le lun. 19 nov. 2018 à 14:49, Julian Hyde <jh...@apache.org> a écrit :
> >>>
> >>>>> On Nov 19, 2018, at 11:19 AM, Vladimir Sitnikov <
> >>>> sitnikov.vladimir@gmail.com> 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
> >>
>
>
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
Posted by Francis Chuang <fr...@apache.org>.
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@apache.org
>
>
> Le lun. 19 nov. 2018 à 15:36, Enrico Olivelli <eo...@gmail.com> a
> écrit :
>
>> Il lun 19 nov 2018, 21:29 Michael Mior <mm...@apache.org> 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@apache.org
>>>
>>>
>>> Le lun. 19 nov. 2018 à 14:49, Julian Hyde <jh...@apache.org> a écrit :
>>>
>>>>> On Nov 19, 2018, at 11:19 AM, Vladimir Sitnikov <
>>>> sitnikov.vladimir@gmail.com> 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
>>
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
Posted by Michael Mior <mm...@apache.org>.
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@apache.org
Le lun. 19 nov. 2018 à 15:36, Enrico Olivelli <eo...@gmail.com> a
écrit :
> Il lun 19 nov 2018, 21:29 Michael Mior <mm...@apache.org> 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@apache.org
> >
> >
> > Le lun. 19 nov. 2018 à 14:49, Julian Hyde <jh...@apache.org> a écrit :
> >
> > >
> > > > On Nov 19, 2018, at 11:19 AM, Vladimir Sitnikov <
> > > sitnikov.vladimir@gmail.com> 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
>
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
Posted by Enrico Olivelli <eo...@gmail.com>.
Il lun 19 nov 2018, 21:29 Michael Mior <mm...@apache.org> 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@apache.org
>
>
> Le lun. 19 nov. 2018 à 14:49, Julian Hyde <jh...@apache.org> a écrit :
>
> >
> > > On Nov 19, 2018, at 11:19 AM, Vladimir Sitnikov <
> > sitnikov.vladimir@gmail.com> 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
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
Posted by Michael Mior <mm...@apache.org>.
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 Mior
mmior@apache.org
Le lun. 19 nov. 2018 à 14:49, Julian Hyde <jh...@apache.org> a écrit :
>
> > On Nov 19, 2018, at 11:19 AM, Vladimir Sitnikov <
> sitnikov.vladimir@gmail.com> 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
>
>
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
Posted by Julian Hyde <jh...@apache.org>.
> On Nov 19, 2018, at 11:19 AM, Vladimir Sitnikov <si...@gmail.com> 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
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
Posted by Vladimir Sitnikov <si...@gmail.com>.
>I don’t think it’s possible to automate.
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.
>I was a bit surprised that you, as a committer, had not already committed
it
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.
PS. It looks like I've posted the above message twice, sorry for
double-posintg.
Vladimir
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
Posted by Julian Hyde <jh...@apache.org>.
I don’t think it’s possible to automate. Crafting good messages is an art, not a science. Which is not to say that developers can’t get good at it, with a little practice.
I’m sorry if you feel intimidated by my critiques. The fact that people can criticize commits after the event is a good thing - it allows us to move faster, because PRs do not have to be made 100% perfect before they are submitted. I try to make my criticism as gently as possible, and bear in mind that everyone has this project’s best interests in mind.
Regarding https://git-wip-us.apache.org/repos/asf?p=calcite.git;a=commit;h=53e15af6c5e8e782b2edcd7f5bf4f5f32225d110. I wish that you had committed that change, and I was a bit surprised that you, as a committer, had not already committed it. It would have saved me 2 hours over the weekend reviewing and revising it. I included both contributors’ names in the message because I had squashed together two commits from different authors. I don’t know whether I broke guidelines, but I was acting in good faith.
Julian
> On Nov 19, 2018, at 10:55 AM, Vladimir Sitnikov <si...@gmail.com> wrote:
>
>> Sorry to harp on to everyone about commit messages. But a clear code
> history is essential for future contributors, and clear release notes are
> essential for our users.
>
> Sorry for highjacking the thread, however it would be really great if the
> verification could be automated.
> The fear of "Julian claiming the commit message not being good enough" is
> very real for me, and it is part of the reason I asked Michael to commit
> "[CALCITE-2266] Implement SQL:2016 JSON functions" (and he was kind enough
> :) ! )
>
> PS. I'm not sure if
> https://git-wip-us.apache.org/repos/asf?p=calcite.git;a=commit;h=53e15af6c5e8e782b2edcd7f5bf4f5f32225d110
> is
> in line with "standards".
> Should committers (e.g. "Vladimir Sitnikov") be placed to the commit
> headline (first line)?
> I thought we don't place committer names to the headline, however the Art
> of Calcite Commit Message is close to black magic to me.
>
> Vladimir
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
Posted by Vladimir Sitnikov <si...@gmail.com>.
>Sorry to harp on to everyone about commit messages. But a clear code
history is essential for future contributors, and clear release notes are
essential for our users.
Sorry for highjacking the thread, however it would be really great if the
verification could be automated.
The fear of "Julian claiming the commit message not being good enough" is
very real for me, and it is part of the reason I asked Michael to commit
"[CALCITE-2266] Implement SQL:2016 JSON functions" (and he was kind enough
:) ! )
PS. I'm not sure if
https://git-wip-us.apache.org/repos/asf?p=calcite.git;a=commit;h=53e15af6c5e8e782b2edcd7f5bf4f5f32225d110
is
in line with "standards".
Should committers (e.g. "Vladimir Sitnikov") be placed to the commit
headline (first line)?
I thought we don't place committer names to the headline, however the Art
of Calcite Commit Message is close to black magic to me.
Vladimir