see my comments inline below but in general +1
Kind regards,
Dave Sag
Brett Porter <[EMAIL PROTECTED]> wrote on 07-03-2006 21:10:53:
> This is not too long, and important. Please read.
>
> I've just spent some time discussing this on users@, and felt it was
> time to bring it here to look at the practical steps going forward.
>
> Basically, I think we've all known for a long time that both need work.
> There was a big push around 2.0 but it fizzled out afterwards.
>
> I don't want to focus on what we are missing now - we can address that
> with what is already in place. I also want to put aside the
> documentation and testing efforts that are already under way as they've
> been taken into consideration. What I want to focus on is going forward
> - new work. I think we need to change the culture. I realise this is
> hard given the timing because there isn't a lot of new work going on -
> because we're all doing bug fixes, docs and testing! - but hopefully
> because of this it will be apparent why it is important to do them as we go.
>
> I've been trying to push for this for a long time, but haven't lived up
> to it myself which is one reason why it fails. I hate hypocrisy so can't
> really call other people on it (though I probably do anyway), and it
> requires everyone to buy in.
>
> So, I've committed r383963 which emphasises these requirements on patch
> contributions:
>
> * Whether it works and does what is intended.
> * Whether it fits the spirit of the project.
> * Whether it contains tests. It is expected that any patches relating to
> functionality will be accompanied by unit tests and/or integration
> tests. It is strongly desired (and will be requested) for bug fixes too,
> but will not be the basis for not applying it. At a bare minimum, the
> change should not decrease the amount of automated test coverage.
> * Whether it contains documentation. All new functionality needs to be
> documented for users, even if it is very rough for someone to expand on
> later. While rough is acceptable, incomplete is not.
>
> In the following, I'll discuss a technical veto. That's a -1 that means
> that the issue needs to be resolved before a release can occur. It
> doesn't mean someone rolls it back immediately (though a release manager
> may if it blocks a branch being released). It also doesn't mean anything
> negative about the committer in question, and it's important we keep it
> that way so that people aren't afraid to contribute or to call people on
> theese things.
>
> So my questions are, can we institute the following:
>
> * new functionality without automated test coverage can and should be
> technically vetoed. We can measure this with code coverage tools, and
> the measure will be that the % does not decline. It will actually break
> the build, being an automatic technical veto :)
>
Yes please. Which code coverage tool will you use? If cobertura then will that mean that the outstanding cobertura plugin bugs will be addressed soon :-)
eg http://jira.codehaus.org/browse/MCOBERTURA-2 and http://jira.codehaus.org/browse/MCOBERTURA-5
> * new functionality without documentation can and should be technically
> vetoed. We can't really measure this with tools, so need to be vigilant
> on it ourselves. We also need to be understanding that docs may follow
> the code once it is finished, so we should use JIRA to track it (keep
> the original ticket open until documented, or create a new, blocking issue).
absolutely YES!
> * new code without javadoc/code documentation can be technically vetoed.
> I'm less tied to this one, though for public facing APIs I think Javadoc
> is imperative. It may be that we can use Checkstyle to enforce this to a
> degree.
I am in favour of being quite hard-core on this. Writing javadocs isn't hard.
> * code that doesn't meet current metrics can and should be technically
> vetoed. Again, we should set these up to fail the build, using
> Checkstyle, PMD and Clirr.
We have found it's also quite useful to pipe the output of checkstyle et al thru QALab so you get some very nice visuals showing metrics vs time.
cheers
dave
