avro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From suraj acharya <suraj....@gmail.com>
Subject Re: Pre-commit Check Avro
Date Tue, 13 Sep 2016 21:57:02 GMT
Hi,
When I reached out to the Apache Yetus dev mailing list for some
information regarding the addition of precommit check.
I got the reply from them which said that AVRO might want to change the
approach of which they run the tests.
I wanted to know if everyone was okay with what was being suggested before
I go around trying to make it work.

What is the opinion of people ?

---------- Forwarded message ----------
> From: Allen Wittenauer <aw@effectivemachines.com>
> Date: Tue, Sep 13, 2016 at 8:13 AM
> Subject: Re: Avro Pre-commit.
> To: dev@yetus.apache.org
>
>
> I've been thinking about this issue.  With a bit of work, I think the
> approach should probably be to:


>         a) make a custom build tool that knows which dirs use which other
> tools
>         b) add a new file in each dir that can tell precommit where to
> break for modules
>
> For example, let's say we have a dir structure like this:
>
>                 build.sh
>                 c/
>                         Makefile
>                 java/
>                         pom.xml
>                 perl/
>                         Makefile.PL
>
> Let's add a file called multibuild.sh that precommit can use:
>
>                 build.sh
>                 multibuild.sh
>                 c/
>                         Makefile
>                         multibuild.sh
>                 java/
>                         pom.xml
>                         multibuild.sh
>                 perl/
>                         Makefile.PL
>                         multibuild.sh
>
>
> In our "multibuild" build tool plugin, we'd define
>
>                 function multibuild_buildfile
>                 {
>                         echo "multibuild.sh"
>                 }
>
>         Now when precommit gets a patch, it will know that a change in c/
> is a different module than a change in java/ or perl/ because of the
> presence of the multibuild.sh file.  This prevents the "build the world"
> problem for a largely diverse project like avro.
>
>         The place where this gets hard is our theoretical
> multibuild_executor function because precommit doesn't pass as a param the
> module that's currently being processed. (we should probably fix that) But
> we can still make it work and reality, it might have led us down a false
> path.  If we make this function look like this:
>
>                 function multibuild_executor
>                 {
>                         echo "multibuild.sh"
>                 }
>
> and make sure that BUILDTOOLCWD=module (the default), then precommit will
> run our file that we placed to actually do the work.  This file can then
> take whatever parameters we're getting passed and convert it as appropriate
> to the build tool for that directory. Since this is a custom build tool,
> you can dictate what parameters to pass to which modules via
> _modules_workers.  For example, an easy route might be:
>
>                 function multibuild_modules_workers
>                 {
>                         modules_workers $2
>                 }
>
> then multibuild.sh for each language becomes a series of case statements.
> For C, it might look like:
>
>                 testname=$1
>                 shift
>
>                 case ${testname} in
>                         compile)
>                                 make -Dcustomflag=1
>                         ;;
>                         doc)
>                                 make man
>                         unit)
>                                 make -Danotherflag=1 test
>                         ;;
>                         *)
>                         ;;
>                 esac
>
>         It's pretty hacky, but theoretically it would work until we get a
> chance to make precommit smarter about multiple build tools in a single
> project. (which would be a pretty hard undertaking, but something we should
> probably do)
>
>
> > On Sep 10, 2016, at 6:32 PM, suraj acharya <suraj.spa@gmail.com> wrote:
> >
> > Yes.
> > The ./build.sh test
> > <https://github.com/apache/avro/blob/master/build.sh#L40> command helps
> run
> > all the tests from the top level.
> > However, I am not sure if we need to run all the tests when a change is
> > made?
> > I was thinking of running language specific tests for changes in the
> > specific directory.
> > For example for changes in python will trigger ant test
> > <https://github.com/apache/avro/blob/master/build.sh#L43>.
>

-Suraj Acharya

On Fri, Aug 19, 2016 at 9:54 AM, Sean Busbey <busbey@cloudera.com> wrote:

> If we have files that fail to include the required ASF licensing
> headers, you're correct that's important to fix.
>
> I think either working on it before or after would be fine. Ideally,
> Yetus Precommit should properly let us know when a patch is fixing
> those kinds of problems, so it would be a good validation that things
> are working.
>
> On Thu, Aug 18, 2016 at 8:18 PM, suraj acharya <suraj.spa@gmail.com>
> wrote:
> > Okay. I will look at the docker command and the image.
> >
> > One other question I have is that the asflicense is an important aspect
> of
> > yetus. However, many java files have that missing. And whenever a patch
> > touches that file it returns a -1. Do you think I should first fix all
> the
> > licenses and then continue with this?
> >
> > S
> >
> > -Suraj Acharya
> >
> > On Thu, Aug 18, 2016 at 12:37 PM, Sean Busbey <busbey@cloudera.com>
> wrote:
> >
> >> is the 3-5 minutes doing the tests across all of the language
> >> libraries or just the java ones?
> >>
> >> docker will definitely be needed, due to the number of different
> >> system dependencies needed to build the different languages.
> >> I was hoping we could reuse the Docker image that is currently
> >> used for the "./build.sh docker" command.
> >>
> >>
> >> On Wed, Aug 17, 2016 at 6:41 PM, suraj acharya <suraj.spa@gmail.com>
> >> wrote:
> >> > Hi,
> >> > I am a new guy and I have been working on setting up pre-commit check
> for
> >> > Avro : AVRO-1887 <https://issues.apache.org/jira/browse/AVRO-1887>.
> >> >
> >> > I was looking at the unit tests being run currently. It takes
> somewhere
> >> > around 3-5 minutes depending on the network and the repo's present.
> >> > I was thinking that since the time is so low why not run all the tests
> >> for
> >> > every commit. Also, the tests being lightweight I was thinking that
> there
> >> > is no need for docker to be needed.
> >> >
> >> > I would like to gather your opinions on this topic before going
> further
> >> > down this path.
> >> >
> >> > Thanks
> >> >
> >> > -Suraj Acharya
> >>
> >>
> >>
> >> --
> >> busbey
> >>
>
>
>
> --
> busbey
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message