Hi, see my comments inline.
On Jul 7, 2010, at 18:32 , David Winslow wrote: > Hey all, > > Thus far the GeoNode has been fairly laxly managed with regard to commit > review, standards enforcement, and basically anything that looks like > watching what goes into the repository. I think as we approach 1.0 we should > revisit these issues to help facilitate participation from "non-core" > contributors. > > recap > Just to review, GeoNode's source tree can be thought of as three or four > different components: > > * A Django application > * Some JavaScript and CSS to decorate the Django application. It's Serious > Stuff and not just a subproject of the web frontend. > * A collection of GeoServer extensions to support the Django application. > * And a bunch of bubblegum and python scripts to stitch it all together. > > Right now Ariel, Ivan, Seb, Andreas, Gabriel, and myself have free commit > access to all of that (and it's all in one git repo so restricting access > partwise would require restructuring things). > > We also maintain a few static resources (sample datadir for GeoServer, > GeoNetwork with custom schema preinstalled) and a few git-svn mirrors of > "upstream" projects (OpenLayers, GeoExt, gxp). Gabriel unofficially > maintains the former, Andreas maintains the latter, and I have access to step > in and update both in case they are unavailable or whatever. > > questions > These are some things that other projects have explicit policies for (which > is nice in avoiding partiality in some potentially touchy decisions) > > How do you prove you are trustworthy enough to commit directly to the main > repository? As in GeoServer, most other projects require you to submit several quality patches before you can become a core committer. For core committers, I'd say we should enforce that every change requires a ticket. The commit comment has to link to the ticket, and when closing the ticket a link to the committed changeset has to be added to the ticket. We should start with this as of now. As soon as possible, we should start requiring unit tests. Selenium has been brought up, which is great, but it's also got quite a learning curve and creating tests is somewhat time consuming. Personally, I'm familiar with TestAnotherWay, which is not great, but does its job well for JavaScript stuff - as long as there is not too much UI to test. In OpenLayers, unit test coverage used to grow with every ticket/patch. So one way to do it would be to start requiring unit tests at some point, and from that point on every change needs to be accompanied by unit tests. > How do you propose changes if you haven't proved that yet (and maybe never > will)? My proposal for GeoNode: Patches from community members without commit access to the main repository require a patch, which is attached to a ticket. Discussion of the change in the mailing list with a reference to the ticket is encouraged. If a core committer is convinced, after several patches, that they show a deep understanding of the code base and architecture, the committer can propose to give the contributor core commit access. We need to decide on a voting process for that. > What criteria are sufficient to reject a patch? This will be much easier when we have unit tests. Patches that make unit tests fail will be rejected. Coding style is also a criteria. And if we keep the concept of domain experts (e.g. myself for the JavaScript part), domain experts usually can judge by their project knowledge if the patch does something that does not fit. If it does not fit, the reason has to be explained on the ticket. > How are major changes (large refactorings, changes to the interface between > components, big new features) handled? Big new features that don't affect existing code can go in with the lightweight process discussed above. For big changes to existing code: discussion on the mailing list, tickets with patches or a separate branch on github. If the change involves changes to more than one domain (e.g. Django and JavaScript), core committers of all affected domains need to review the patch/branch before it can go into the main repo. All these questions need to be answered, and the processes described, on a community/process page on the project wiki. Another thing that is important: if OpenPlans is to be the copyright holder, we will need contributor license agreements from all contributors and committers. My 2ยข, Andreas. > > case study > In GeoServer, which is both fairly familiar to me and a good example of a > project with intentionally distributed decision making, the answers are: > > * Prove you are trustworthy by: Get at least two accepted patches into SVN, > and receive a nomination from a current committer. Two plus votes are also > required, and any committer can veto. > > * Propose changes by: Report an issue on GeoServer's JIRA setup. Discussion > on the mailing list beforehand, and attaching a patch both encouraged but > optional. Committers are expected to go through JIRA as well to help track > what improvements go into each release. > > * Reject a patch: There are a number of guidelines; style guidelines about > how to indent code and such, design guidelines about how geoserver code > should interact with geotools code, etc. GeoServer is also divided up into > several modules each of which has a "maintainer" who reviews patches and can > dismiss a patch for any reason he sees fit. > > * Large refactorings or new features require a GSIP (GeoServer Improvement > Proposal) before they can be committed to SVN. These are voted on by the PSC > after a "review and revise" period that lasts a few days. > > current status > For us right now, the policy is something like: > > * Prove you are tsustworthy by being around early in the project history > * Propose changes by: discussion on the mailing list. We don't have a lot of > changes coming in from non-committers. > * Reject a patch: hasn't come up yet because everyone working on GeoNode > right now has commit rights. > * large refactorings or new features - git branching is encouraged but not > enforced. > > bottom line > So what should the geonode policy be? some goals of the policy would be > - guide arbitration on third party (and 'core') changes > - maintain some level of code quality in 'master' > - facilitate production of especially-well-tested releases (compared to > master at an arbitrary point in time) > - facilitate development of custom features/extensions to the geonode > > -- > David Winslow > OpenGeo - http://opengeo.org/ -- Andreas Hocevar OpenGeo - http://opengeo.org/ Expert service straight from the developers.
