Hi Kevin, Woo! Glad to read this mail this morning, it is a very good start :) BTW seems to be related to SHINDIG-261.
Let's comment it and adding some reasoning, similar to the javastyle 2008/5/10, Kevin Brown <[EMAIL PROTECTED]>: > Hi everyone, > > I'm trying to gather some additions to the contributors page of the shindig > site. I'd like to address a few issues that have come up with patches from > non-committers. Here's what I have so far: > > - Patches should be attached to the JIRA issue tracker. Patches sent via > email will likely just be forgotten. Definitely! Reasoning: traceability, issue tracking in one place > - Patch must conform to style guides as specified by > http://cwiki.apache.org/confluence/display/SHINDIGxSITE/Java+Style, > http://cwiki.apache.org/confluence/display/SHINDIGxSITE/Javascript+Style, > http://cwiki.apache.org/confluence/display/SHINDIGxSITE/PHP+Style, etc. > Minor deviations are acceptable, at the discretion of the committer (who > must resolve the deviations herself). If a commiter judges that a patch is > too far off of the style guide, she may reject it. The main point here is "the discretion of the committer". That should not be a restriction on patch acceptance: any committer could easily run ctrl+shift+F in their favorit IDE to format patch code... Reasoning: easy integration, patch readingness > - Any new code paths should include corresponding unit tests. New classes > with any non-trivial public methods (i.e. anything that actually does work) > must provide a matching test case. No patch will be accepted without > corresponding test coverage. There is currently an exception to this for > javascript, pending a solid solution for running jsunit or similar tests. As above, the committer has the responsibility to accept or reject a patch. Any rejects should be explain. Reasoning: easy integration, quality, testing > - All existing tests must pass after the patch is applied. If any tests > fail, you must also patch the code that is now failing (either by updating > the test, or modifying the code path as appropriate). If you need to make > any major changes to other code that is not directly related to your change, > please include notes to that effect in the JIRA ticket. If you're unsure of > what to do, email shindig-dev@, or the component lead for the code you're > patching. As above, the committer has the responsibility to verify that no regression was done with a given patch. Also, users should create more tickets for other code and link them. Reasoning: quality, regression, decrease side effects > - Patches should be as small and focused as possible. The smaller they are, > the easier it is to avoid conflicts and for committers to review them. IMHO this is an important point: patches should be focused, ie no include any part of code like formatting/cleaning/whatever on the non related issue (like said, users could always create other issues for that) Reasoning: traceability, decrease side effects > - Patch should be against a relatively recent version of Shindig so as to > avoid merge conflicts; ideally, the patch should be against HEAD at the time > of creation. Committers will do our best to resolve any minor merge issues > that occur between patch submission and commit time, though we may not be > able to do this in the event of major conflicts. In fact, only if committers can't integrate patch Reasoning: easy integration > - Any changes in javascript with implications for server side functionality > must be approved by all server-side language implementors. Committers are > responsible for ensuring that appropriate language components are assigned > to the issue so that they are properly tracked. Reasoning: quality, api > - Committers must reference the issue when committing the patch. The ideal > format is something like "Applying SHINDIG-XXX, submitted by <name of user > that submitted it". This will allow automatic status updates for the > associated JIRA tickets. IMHO it is important to have this to create an open and diverse community. Standard commit message helps to recruit users/dev/committers/pmc Moreover, it improves the svn integration in jira Reasoning: quality, integration, tracking informations (specially potential committers) I will add other tips: - as they can, committers should commit patches "as is" and commit after any changes like formatting, improvements... Reasoning: traceability, quality, legal - committers should commit issue after issue to make small commits: forbid all in one patch (ie 652498) BTW small commits are always better. Reasoning: traceability, patch readingness - users/committers should use Jira to track all comments: no mail replies on a Jira issue on [EMAIL PROTECTED] Comments are only in one place. If an issue needs a discussion on dev@ or on wiki space, users/committers have the responsibility to add a link to jira. Reasoning: traceability Cheers, Vincent
