One thing I can think of right off the bat that is comments on the Python stuff in particular. I think it would be good to adopt a standard for that and try to stick to it.
On Thu, Jul 8, 2010 at 3:00 AM, Andreas Hocevar <[email protected]>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/ > > -- > Andreas Hocevar > OpenGeo - http://opengeo.org/ > Expert service straight from the developers. > > -- Sebastian Benthall OpenGeo - http://opengeo.org
