Thanks for your input, guys! I see my position is not really distracted from yours, I'm happy with that :)
Regards, Aleksey. On Fri, Apr 25, 2008 at 9:19 AM, Alexey Varlamov <[EMAIL PROTECTED]> wrote: > 2008/4/25, Nathan Beyer <[EMAIL PROTECTED]>: > > > On Thu, Apr 24, 2008 at 10:34 AM, Aleksey Shipilev < > > [EMAIL PROTECTED]> wrote: > > > > > Hi Alexey (Petrenko), Nathan, Mark, all! > > > > > > Thanks for supporting me in creating the good patches against Harmony! > > > I had read "Good Issue Resolution Guideline" [1] and "Bad Smells" [2]. > > > I will struggle to comply with them further. But also I need to know > > > the answers on topics which are not covered by the docs. I have my own > > > answers on them, but it would be great to see if my perception is what > > > Harmony committers expect. > > Thanks for raising this theme Aleksey! > Common short answer is: it should be practical and convenient, don't > think formally. > More detailed comments inlined. > > > > > > > > 1. Coding conventions. What coding convention Harmony is using? Is it > > > common "Code Conventions for the Java" [3]? > > > > > > Yes and no. For the most part, these cover the basic formatting style used. > > On the other hand, always respect the format that has already been > > established, unless is ridiculously out of whack. > +1 > Usually it is a good practice to use any decent tool/plugin as you > write your code, this helps to get a taste of style. It is of lesser > importance which particular conventions are used (just appropriate for > used programming language), but style should be consistent - so as to > be easily readable and less confusing. Therefore it is preferable to > avoid unmotivated change of style in a fixed code. > > > > > > > > > > > > > > > 2. Patch separation. I see that there is an requirement in place to > > > divide test and implementation parts into distinct patches. Does that > > > apply to formatting, documentation and > > > text-reordering patches? I guess, it does. > > > > > > > I think there are various approaches and everyone has a slightly different > > preference. I like fewer patches, but that's general because I prefer very > > targeted changes, which generally aren't large. For bugs, it's nice to have > > a test patch that can be applied and show the failure, which is separate > > from the fix patch. > Yes, this is a matter of usability again. Try thinking how it would > look for a guy which deciphers commit logs in a year: > formatting/reordering changes camouflage semantical bits thus should > be separated. OTOH documention accompanying modified business logic > (which simplifies understanding) fits nicely to the same patch. > > > > > > > > > > > > 3. Explaining what the patch does. To what extent should one describe > > > what exactly the patch does: should it be the "investigation path" > > > which lead to fix, short description of fundamental changes, the > > > reason why this exact solution is better? I think that short > > > description on each change in patch should be enough. > > > > > > Bugs - describe the bug, why it's a bug and how the bug is fixed > > Enhancement - describe what it is, describe the value (don't assume it's > > self-evident) > > Yep, it depends on patch. Basically it should be convincing for > committers and fellow contributors which review your patches. Yet it > is a good way to share your unique bits of knowledge - e.g. for me, > this is a great fun to learn something new while reviewing issues. So > it is really helpful for others when you describe how you've chased > some tricky issue. > > > > > > > > > > > > > 4. Proof-of-concept patches. Is it acceptable to have POC patches > > > attached to JIRA if there's distinct reminder that the patch is > > > prototype and is not supposed to be committed? Personally I like to > > > attach the "checkpoint patches" to JIRA when I'm working on issues, > > > 'cause this could minimize the efforts on continuing works in case of > > > something happens with my computer or me :) > > > > > > > Personally, I don't think POC work should be put into JIRA, at least not in > > general. This is just normal development and it should be brought up in the > > mailing list. A POC, in essence is just an idea that's illustrated with > > code. Ideas have to be discussed and communicated before they can make it > > into the code base and the only place to do that is the mailing list. > > I would argue here. Code is always the best illustration of an idea, > and it is more convenient to review and refer to if materials are > accumulated in one place. > There's nothing wrong to have non-commitable patches in JIRA if you've > clearly stated purpose of the patch. > > Regards, > Alexey > > > > > > > > > > > I think the answers on these questions worth publishing in resolution > > > guideline or similar document. > > > > > > Is there anything else to read about good coding practices? > > > > > > Thanks! > > > Aleksey. > > > > > > [1] http://harmony.apache.org/issue_resolution_guideline.html > > > [2] > > > http://sis36.berkeley.edu/projects/streek/agile/bad-smells-in-code.html > > > [3] http://java.sun.com/docs/codeconv/html/CodeConvTOC.doc.html > > > > > >
