Mailing-List: contact dev-help@ambari.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@ambari.apache.org
Subject: Design & Code Review Guidelines
MIME-Version: 1.0
From: Alejandro Fernandez <afernandez@hortonworks.com>
To: "dev@ambari.apache.org" <dev@ambari.apache.org>
Thread-Topic: Design & Code Review Guidelines
Thread-Index: AQHRhhvsfbjFMUDZBE2FLnagL/1Drw==
Date: Thu, 24 Mar 2016 22:24:26 +0000
Message-ID: <D319B727.62D72%afernandez@hortonworks.com>
Accept-Language: en-US
Content-Language: en-US
user-agent: Microsoft-MacOutlook/14.4.4.140807
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [192.175.27.10]
x-source-routing-agent: Processed
Content-Type: multipart/alternative;
	boundary="_000_D319B72762D72afernandezhortonworkscom_"

--_000_D319B72762D72afernandezhortonworkscom_
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable

Hi all,

I wanted to send a refresher on good design and code review guidelines.

Creating Jiras:
If you want your Jira to get attention and be resolved, please provide suff=
icient details: version found on, ambari-server --hash, database dump, repr=
o steps, stack trace, etc.
This can also help someone determine if an issue has already been fixed on =
a newer version.

Further, make sure that the Jira title addresses the root cause and not an =
implementation proposal. At times, we have a tendency to identify a solutio=
n, and in doing so the original problem and root cause get lost; by the tim=
e someone works on the Jira, they may end up adding superfluous code if the=
 underlying code base has changed or provides a better way to solve the pro=
blem.

Design Reviews:
For large features, we must circulate design reviews in the community. This=
 gives others a chance to comment and provide insight into how a feature ca=
n reuse existing designs or improve upon it.
Sometimes the best design is actually really simple. For extremely large fe=
atures, it's ok to make a feature branch, write the code and stabilize it, =
and then integrate to trunk.

Code Reviews: https://smartbear.com/learn/code-review/best-practices-for-pe=
er-code-review/

  1.  Authors should annotate source code before the review. This makes it =
easier for devs reviewing your code and may even help you spot bugs before =
they do.
  2.  Send small code-reviews if possible. Reviewing more than 400 lines pe=
r hour diminishes our ability to find defects.
  3.  Reviewing code for more than one hour also reduces our ability to fin=
d bugs.
  4.  If possible, try to break up large reviews into separate but function=
al stages. If you need to temporarily comment out unit tests, do so. Sendin=
g gigantic patches means your review will take longer since reviewers need =
to block out more time to go through it, and you may spend more time revvin=
g iterations and rebasing.

We have a global community of committers, so please be mindful that you sho=
uld wait at least 24 hours before pushing your patch even though you may al=
ready have the necessary +1.
This encourages others to take an interest in your review and helps us find=
 more bugs (it's ok to slow down in order to speed up).

If you have any suggestions, feel free to share.

Thank you,
Alejandro Fernandez

--_000_D319B72762D72afernandezhortonworkscom_--