couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jason Smith <>
Subject Re: A Plan: Remove pre-commit, jshint, code style requirements from couchdb-nano
Date Wed, 16 Sep 2015 00:26:50 GMT
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

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 <> 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 <>
> wrote:
> > Hi, Dale.
> >
> > On Tue, Sep 15, 2015 at 7:26 PM, Dale Harvey <>
> 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.)
> >

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