Mailing-List: contact dev-help@couchdb.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@couchdb.apache.org
MIME-Version: 1.0
In-Reply-To: 
 <CAJ1bcfF_WW+cZyNYPxqu4e5AW7SfM4oLHiXUMxE8V_uutbbmZg@mail.gmail.com>
References: 
 <CACLy94WwEdD9ib+9kpy5U5Ov2CFoSaYo-5EzmMWnomBg-AbCZQ@mail.gmail.com>
 <CAD2UGCWv9DqU=ZMr4MyZ1HeyK2n_J6fbwUz5+0F1uu1iUGc0ZA@mail.gmail.com>
 <CACLy94V592nh=KGdQt1CokS35SGm+evT7frvHpVPFMTWDEs+rw@mail.gmail.com>
 <CAJ1bcfF_WW+cZyNYPxqu4e5AW7SfM4oLHiXUMxE8V_uutbbmZg@mail.gmail.com>
From: Jason Smith <jason.h.smith@gmail.com>
Date: Wed, 16 Sep 2015 07:26:50 +0700
Message-ID: 
 <CACLy94U==M0qCApk+si8n8gjR7DudQTvJGmcX+AHx--SWq_Ncg@mail.gmail.com>
Subject: Re: A Plan: Remove pre-commit, jshint,
 code style requirements from couchdb-nano
To: "dev@couchdb.apache.org" <dev@couchdb.apache.org>
Content-Type: multipart/alternative; boundary=089e0149cbe8edbb18051fd25b4e

--089e0149cbe8edbb18051fd25b4e
Content-Type: text/plain; charset=UTF-8

Hi, Robert. I agree with everything you said.

Furthermore, I would like to adopt the Fauxton style for the Nano project
(and I propose that it extends to all JavaScript under the CouchDB
umbrella).

As a first step, I will simply remove the tests from pre-commit hooks. In
the future, I would like to re-use or duplicate your CI tools for Nano. But
first thing's first! I want to fix a couple of bugs and ship a release.

On Wed, Sep 16, 2015 at 7:18 AM, Robert Kowalski <rok@kowalski.gd> wrote:

> Hey there,
>
> just as a report how we are doing it with Fauxton:
>
> the Fauxton project also has a styleguide in order to make reviews and
> reading code easier for everyone. At one point it turned out to reduce a
> lot of work for the reviewers if we test automatically for the coding style
> as part of our testsuite.
>
> We don't use commit hooks for it, but the CI will fail if the automatic
> coding style check and also the testsuite was not successful. the style
> check is runs before the unit tests run, which run before the integration
> tests. Red tests (that include tests for style) are a no-go for a merge in
> Fauxton.
>
> I don't think the checks must be enforced on a pre/post-commit-hook-level
> but somewhere they should notify the proposer of the change to make the
> live of the reviewer easier - in case you think code style is important for
> your project. I also want to not that some sub projects of CouchDB have no
> styleguide at all and that it can be a positive thing or a negative thing.
> Having one worked out well for Fauxton.
>
> I also think that deploying with a red CI might be OK in exceptional cases,
> but in these cases the deploying person should be aware why the test fails
> in 100%, and know the reasons why it is not fixable right now.
>
> In the recent past I have seen people saying they would know why the
> integration tests fails, delivering buggy releases.
>
> I think it is a small boundary and not everyone who says they know why the
> tests fail is 100% sure why they really fail (and deploys broken releases
> therefore). But I have worked with people that were so deep into the
> testsuites and the project they maintained in the past that they were able
> to predict that.
>
> In short I support whatever you decide for I just wanted to share my
> experiences, maybe it helps :)
>
> On Tue, Sep 15, 2015 at 2:54 PM, Jason Smith <jason.h.smith@gmail.com>
> wrote:
>
> > Hi, Dale.
> >
> > On Tue, Sep 15, 2015 at 7:26 PM, Dale Harvey <dale@arandomurl.com>
> wrote:
> >
> > >
> > > I just wanted to clarify, are you speaking about removing as a
> > "pre-commit
> > > hook", or removing the requirements for those checks to pass before
> > > merging?
> > >
> >
> > I am speaking about removing the pre-commit hook only--the mechanical
> thing
> > that that runs automatically when one commits.
> >
> > The tests and checks would make brilliant push hooks, or perhaps Travis
> > tests for pull requests. However they are a bit much as a *commit* hook.
> >
> > A separate conversation: should the tests pass for merging. I would say:
> > yes if it's smart; no if it's dumb: they need not pass for merging, at
> > least not automatically, mechanically. The reason is that we should merge
> > pull requests liberally, accepting contributions from all, then commit on
> > top of those to correct for style. I won't have some sort of angry
> > Calvinist robot telling me I can't merge a pull request. (If we can be
> > clever, for example to require all tests pass for the master branch but
> not
> > feature branches, then yes I would love that.)
> >
>

--089e0149cbe8edbb18051fd25b4e--