harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexey Varlamov" <alexey.v.varla...@gmail.com>
Subject Re: [general] How to be a good contributor?
Date Fri, 25 Apr 2008 05:19:41 GMT
2008/4/25, Nathan Beyer <ndbeyer@apache.org>:
> On Thu, Apr 24, 2008 at 10:34 AM, Aleksey Shipilev <
> aleksey.shipilev@gmail.com> 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.
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.


> >
> > 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
> >

View raw message