Rather than do this...

if ( foo.equals( bar ) ) {     // don't do this

... do this instead...

if (foo.equals(bar)) {

Also, rather than do this...

foo = barArray[ i ];     // don't do this

... do this instead...

foo = barArray[i];

Auto-generated code in Eclipse often looks like this...

 public void readFields(DataInput arg0) throws IOException {    // don't do this
   foo = arg0.readUTF();                                       // don't do this

... do this instead ...

 public void readFields(DataInput di) throws IOException {
   foo = di.readUTF();

See the difference? 'arg0' is what Eclipse uses for arguments by default.

Keep lines less than 100 characters.

Bar bar = foo.veryLongMethodWithManyArguments(argument1, argument2, argument3, argument4, argument5, argument6, argument7, argument8, argument9);  // don't do this

... do something like this instead ...

Bar bar = foo.veryLongMethodWithManyArguments(
 argument1, argument2, argument3,argument4, argument5, argument6, argument7, argument8, argument9);

This happens more than people would imagine.

Bar bar = foo.getBar();     <--- imagine there's an extra space(s) after the semicolon instead of a line break.

Make sure there's a line-break after the end of your code, and also avoid lines that have nothing but whitespace.

Every class returned by RegionServers must implement Writable. If you are creating a new class that needs to implement this interface, don't forget the default constructor.

This is also a very common feedback item. Don't forget Javadoc!

Javadoc warnings are checked during precommit. If the precommit tool gives you a '-1', please fix the javadoc issue. Your patch won't be committed if it adds such warnings.

Findbugs is used to detect common bugs pattern. As Javadoc, it is checked during the precommit build up on Apache's Jenkins, and as with Javadoc, please fix them. You can run findbugs locally with 'mvn findbugs:findbugs': it will generate the findbugs files locally. Sometimes, you may have to write code smarter than Findbugs. You can annotate your code to tell Findbugs you know what you're doing, by annotating your class with:

@edu.umd.cs.findbugs.annotations.SuppressWarnings(
                    value="HE_EQUALS_USE_HASHCODE",
                    justification="I know what I'm doing")

Note that we're using the apache licensed version of the annotations.

Don't just leave the @param arguments the way your IDE generated them. Don't do this...

  /**
   *
   * @param bar             <---- don't do this!!!!
   * @return                <---- or this!!!!
   */
  public Foo getFoo(Bar bar);

... either add something descriptive to the @param and @return lines, or just remove them. But the preference is to add something descriptive and useful.

If you submit a patch for one thing, don't do auto-reformatting or unrelated reformatting of code on a completely different area of code.

Likewise, don't add unrelated cleanup or refactorings outside the scope of your Jira.

Make sure that you're clear about what you are testing in your unit tests and why.

If a committer commits a patch it is their responsibility to make sure it passes the test suite. It is helpful if contributors keep an eye out that their patch does not break the hbase build and/or tests but ultimately, a contributor cannot be expected to be up on the particular vagaries and interconnections that occur in a project like hbase. A committer should.