On 07/08/2010 03:00 AM, Andreas Hocevar wrote: > 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/ >> I started a page on etherplans (a clone of Etherpad run by OpenPlans) to follow this discussion and work on a draft of the wiki page. I think it's easier to deal with than interleaving comments in emails like this, plus, you know, it's awesome for collaboratively drafting text.
link: http://etherplans.org/geonode-commit-policy -- David Winslow OpenGeo - http://opengeo.org/
