We do not want the SCM API plugin to become a grab-bag of many different implementations with multiple options being presented to every user.
We want people to be able to scratch their own itch, by writing an extension plugin that supports their desired behaviour.
- Extension plugins can be small and easy to test, especially if they just add one or two
SCMTrait
implementations.- Reduces risk of upgrade of the extension plugin (as there is less changes to assess)
- Reduces risk to release the more core plugins (as there is less functionality exposed by default to users)
In this regard, for SCM API we will:
-
Reject Pull Requests that add new
SCMTrait
implementations unless an exceedingly strong case can be made for their inclusion (i.e. these must be essential for 99.9% of users). Note: thejenkins.scm.impl.trait.*FilterTrait
implementations would not pass this test now. The long term goal is to move these to a separate plugin, they were only added here because a filter was needed for the test harness. -
Approve Pull Requests that enable extension plugins to be written, subject to presenting a coherent design within the overall principles of the SCM API.
If you are writing an extension plugin, please consult the recommendations on naming extension plugins
Truths which we believe to be self-evident (adapted from TextSecure's)
- The answer is not more options. If you feel compelled to add a preference that's exposed to the user, it's very possible you've made a wrong turn somewhere.
- There are no power users. The idea that some users "understand" concepts better than others has proven to be, for the post part, false. If anything, "power users" are more dangerous than the test, and we should avoid exposing dangerous functionality to them.
- If it's "like PGP," it's wrong. PGP is our guide for what not to do.
- It's an asynchronous world. We wary of anything that is anti-asynchronous: ACKs, protocol confirmations, or any protocol-level "advisory" message.
- There is no such thing as time. Protocol ideas that require synchronized clocks are doomed to failure.
- To the largest extent possible, all fields shall be private. Use an IDE to generate the getters and setters.
- If a class has more than one
volatile
member field, it is probable that there are subtle race conditions. Please consider where appropriate encapsulation of the multiple fields into an immutable value object replace the multiplevolatile
member fields with a singlevolatile
reference to the value object (or perhaps better yet anAtomicReference
to allow forcompareAndSet
- if compare-and-set logic is appropriate). - If it is
Serializable
it shall have aserialVersionUID
field. Unless code has shipped to users, the initial value of theserialVersionUID
field shall be1L
.
- Use spaces. Tabs are banned.
- Java blocks are 4 spaces. JavaScript blocks as for Java. XML nesting is 2 spaces
- "hungarian"-style notation is banned (i.e. instance variable names preceded by an 'm', etc)
- If the field is
static final
then it shall be named inALL_CAPS_WITH_UNDERSCORES
. - Start variable names with a lowercase letter and use camelCase rather than under_scores.
- Spelling and abbreviations: If the word is widely used in the JVM runtime, stick with the spelling/abbreviation in the JVM runtime, e.g.
color
overcolour
,sync
oversynch
,async
overasynch
, etc. - It is acceptable to use
i
,j
,k
for loop indices and iterators. If you need more than three, you are likely doing something wrong and as such you shall either use full descriptive names or refactor. - It is acceptable to use
e
for the exception in atry...catch
block. - You shall never use
l
(i.e. lower caseL
) as a variable name.
To the greatest extent possible, please wrap lines to ensure that they do not exceed 120 characters.
- The
pom.xml
file shall use the sequencing of elements as defined by themvn tidy:pom
command (after any indenting fix-up). - If you are introducing a property to the
pom.xml
the property must be used in at least two distinct places in the model or a comment justifying the use of a property shall be provided. - If the
<plugin>
is in the groupIdorg.apache.maven.plugins
you shall omit the<groupId>
. - All
<plugin>
entries shall have an explicit version defined unless inherited from the parent.
- For fields, the order is:
- public / protected / private
- static
- final
- transient
- volatile
- For methods, the order is:
- public / protected / private
- abstract
- static
- final
- synchronized
- native
- strictfp
- For classes, the order is:
- public / protected / private
- abstract
- static
- final
- strictfp
- For code in
src/main
:*
imports are banned.static
imports are strongly discouraged.static
*
imports are discouraged unless code readability is significantly enhanced and the import is restricted to a single class.
- For code in
src/test
:*
imports of anything other than JUnit classes and Hamcrest matchers are banned.static
imports of anything other than JUnit classes and Hamcrest matchers are strongly discouraged.import static org.hamcrest.Matchers.*
,import static org.junit.Assert.*
are expressly permitted. Any otherstatic
*
imports are discouraged unless code readability is significantly enhanced and the import is restricted to a single class.
- Annotations on classes, interfaces, annotations, enums, methods, fields and local variables shall be on the lines immediately preceding the line where modifier(s) (e.g.
public
/protected
/private
/final
, etc) would be appropriate. - Annotations on method arguments shall, to the largest extent possible, be on the same line as the method argument (and, if present, before the
final
modifier)
- Each class shall have a Javadoc comment.
- Each field shall have a Javadoc comment.
- Unless the method is
private
, it shall have a Javadoc comment. - When a method is overriding a method from a super-class / interface, unless the semantics of the method have changed it is sufficient to document the intent of implementing the super-method's contract with:
/** * {@inheritDoc} */ @Override
- Getters and Setters shall have a Javadoc comment. The following is preferred
/** * The count of widgets */ private int widgetCount; /** * Returns the count of widgets. * * @return the count of widgets. */ public int getWidgetCount() { return widgetCount; } /** * Sets the count of widgets. * * @param widgetCount the count of widgets. */ public void setWidgetCount(int widgetCount) { this.widgetCount = widgetCount; }
- When adding a new class / interface / etc, it shall have a
@since
doc comment. The version shall beFIXME
to indicate that the person merging the change should replace theFIXME
with the next release version number. The fields and methods within a class/interface (but not nested classes) will be assumed to have the@since
annotation of their class/interface unless a different@since
annotation is present.
- Eclipse, by and large the IDE defaults are acceptable with the following changes:
- Tab policy to
Spaces only
- Indent statements within
switch
body - Maximum line width
120
- Line wrapping, ensure all to
wrap where necessary
- Organize imports alphabetically, no grouping
- Tab policy to
- NetBeans, by and large the IDE defaults are acceptable with the following changes:
- Tabs and Indents
- Change Right Margin to
120
- Indent case statements in switch
- Change Right Margin to
- Wrapping
- Change all the
Never
values toIf Long
- Select the checkbox for Wrap After Assignment Operators
- Change all the
- Tabs and Indents
- IntelliJ, by and large the IDE defaults are acceptable with the following changes:
- Wrapping and Braces
- Change
Do not wrap
toWrap if long
- Change
Do not force
toAlways
- Change
- Javadoc
- Disable generating
<p/>
on empty lines
- Disable generating
- Imports
- Class count to use import with '*':
9999
- Names count to use static import with '*':
99999
- Import Layout
- import all other imports
- blank line
- import static all other imports
- Class count to use import with '*':
- Wrapping and Braces