ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alejandro Fernandez <>
Subject Design & Code Review Guidelines
Date Thu, 24 Mar 2016 22:24:26 GMT
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 sufficient details:
version found on, ambari-server --hash, database dump, repro 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 solution, and in doing so the original
problem and root cause get lost; by the time 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 problem.

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 can reuse existing designs or improve
upon it.
Sometimes the best design is actually really simple. For extremely large features, it's ok
to make a feature branch, write the code and stabilize it, and then integrate to trunk.

Code Reviews:

  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 per hour diminishes
our ability to find defects.
  3.  Reviewing code for more than one hour also reduces our ability to find bugs.
  4.  If possible, try to break up large reviews into separate but functional stages. If you
need to temporarily comment out unit tests, do so. Sending 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 revving iterations and rebasing.

We have a global community of committers, so please be mindful that you should wait at least
24 hours before pushing your patch even though you may already 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

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message