Release Commit Rules

This Wiki is read-only for reference purposes to avoid broken links.

ABSTRACT

This document is intended to establish written and clear rules to include contributions in official ADempiere releases.

MOTIVATION

Pursuing the mandate of the PMC Head Concept, specifically on the items:

3.2.2 PMC Head must document the rules for migrating commits from trunk to release branch

4.1.1 Protect the stability of Releases

4.1.5 Ensure that additional functionality is discussed and reviewed appropriately from both a functional and technical standpoint

4.2.6 Establish the development processes and infrastructure needed for the development Team to be effective

4.2.10 Establish rules and guidelines for QA

4.2.11 Maintain branch rules

4.2.13 Manage release policies for:

4.2.13.1 New features under development

4.2.13.2 high risk bug fixes

4.2.13.3 high risk enhancements

4.2.13.4 experimental stuff

4.2.14 oversee merging proven stuff from trunk into release branch

4.2.15 reverting when something goes wrong in release branch

The PMC Head is stating here the rules to integrate new things in official Adempiere releases.

MAINTENANCE OF THE RULES

The official maintainer of this set of rules is the PMC Head (at this moment Carlos Ruiz).

The rules are not written in stone – any community member can ask to PMC Head to explain the reason of any of the rules, and recommend a review if there are some disagreement, and any community member can ask to add new rules also in case it's detected a possibility of improving quality adding a new rule.

Please discuss changes on this document (unless it's just style improvements) on forums or discussion tab here.

COMMITTING

A list of committers with permission to write in /release is maintained and published in Adempiere project. You can consult the list of COMMITTERS here.

These rules are intended to be applied when committing on /release. To commit on /trunk a different set of rules can apply, maintained and established by community. Rules to commit in a branch are established by the corresponding branch owner.

When committing it is important to observe the current set of rules to avoid unnecessary reverts and discussions. To conduct experiments any committer can use trunk (according to trunk rules) and/or open a specific branch and experiment there.

REVERTING

The rules are classified in three groups:

• strict-mandatory (S)
• bearable-mandatory (B)
• recommended (R)

Reverts can be applied this way:

• Breaking a strict-mandatory rule (marked as S) will conduct to an immediate revert, any committer can do the revert, to make the revert a log message indicating the broken rule is enough – it doesn't need to be explained, discussed or justified beyond pointing to the broken rule. If the developer being reverted consider it unfair it can be kindly discussed in forums or corresponding tracker. In case of disagreement PMC Head will take the final decision. There can be exceptions when developer is reachable and eager to fix the mandatory-rule breakage, in this case the strict-mandatory rule could be considered as bearable by PMC Head.

• Breaking a bearable-mandatory rule (marked as B) would not conduct to an immediate revert – a kind message about the broken rule will be written to the developer allowing him (or another peer) to fix the problem. The problem must be fixed within a period of 2 days after reported, if not fixed then the commit will be reverted and it can be recommitted after solving the problem. The established 2-day period can become shorter on the days where a release is being prepared.

• Breaking a recommended rule (marked as R) doesn't conduct to a revert, it's recommended the issue to be documented as a TODO.

RULES

The current set of rules is mostly based on the agreed ADempiere Best Practices.

GOOD JUDGMENT

J1 - Don't drop things - always assume there can be others using that (S)

Exception to this rule are things added after the last official release, in this case we can consider dropping them if they were added wrongly.

If you move a column from one table to a different one - then a migration script must take care of migrating the data before dropping the original.

J2 - Always review collateral effect of changes (S)

When you change a method, please check collaterals. Eclipse helps if you right-click over the method name > References > Project (or Workspace)

Also, when changing processes, callouts, and other dictionary mentioned classes, you must check in the database (normally Adempiere_pg.dmp is easy to search for that).

When changing dictionary common entries, like Element, Dynamic Validation, References, you need to check where else the entry is used and verify you don't break other functionalities.

J3 - Add general things on /release, don't add localizations or customizations (B)

Localizations and customizations normally can be added to /release after making them enoughly configurable to be widely general.

COMMUNICATION

C1 - Always discuss new features in forums before you commit (S)

Wait at least three days after opening your thread to allow others to read your message.

Ask and/or announce in forums - always (exception for little bugs).

If you're sure about the fixing of a bug, then please commit, in doubt please ask for previous peer review on forums.

After enough discussion, open a tracker.

C2 - Try to keep all discussion just in forum channel (R)

Trackers are not intended to conduct discussions or collect opinions. Preferrably keep discussion on forums. Correlate tracker and forum adding message links on both.

C3 - Trackers must be descriptive (B)

One-line trackers are not sufficient to allow others to complete a meaningful review of your work. If you're fixing a bug you should provide steps to reproduce the bug in GardenWorld or System. If it's not easily reproducible in GardenWorld, explain in detail what the issue is, in which scenarios the issue appears and the proposed solution (if any). If you are reporting a feature request also provide a suggested use case. When you have implemented the feature request you must then explain how to use the new functionality. Larger changes are better documented in a a wiki page that also provides end user guidance. It is not acceptable to leave it up to others to "read the code" to work out how your contribution works.

C4 - Comments related to commits must be written in the corresponding tracker (R)

Please don't use the maillist to comment about commits. Use the corresponding tracker.

C6 - When in disagreement you can call for a vote (R)

Technical issues must be voted by technical people. Functional issues must be voted by functional people. The voting result cannot override the current set of rules, for example: a positive vote cannot push to /release things working just on oracle.

RESPONSIBILITY

R2 - Branding, IP and copyright (S)

No vendor branding on Adempiere windows, processes and forms.

Your contribution must be proven to be original work, or GPL-compatible work.

INTEGRITY

I2 - Migration script for all databases supported (S)

Oracle and Postgresql support (and any database defined in future as supported by Adempiere project)

Migration scripts on oracle and postgresql must be corresponding, don't edit one script without applying the same changes to the other script.

I3 - Code written to work on the java version supported (S)

At this moment Java 6, this is, you cannot add code that just run on java 7

I5 – Generate X_ classes when needed (B)

When modifying or adding tables and columns, or modifying a validation list, generate the corresponding X_ classes. Use only official ModelGenerator

I7 - Unit tests (R)

It's recommended to contribute unit tests classes when possible and applicable to the code you're contributing.

I9 - Regenerate serialVersionUID when required (B)

Some classes use a variable serialVersionUID that needs to be changed (regenerated) on certain changes. Please check on this link the changes that require regeneration, and recreate the variable when required.

I10 - Be careful with security recommendations (S)

Take into account the best practices on security recommendations for java, SQL, databases, etc.

For example, avoid concatenating Strings within SQL statements - it opens the door for SQL Injection [1]

I11 - Don't use function based indexes to implement validations (S)

Because not all databases support function based indexes - validations must be implemented in java code (or dictionary) - it's not allowed to implement (in official seed) validations using FBIs.

However it can be allowed to include selected FBIs that can have great impact on performance.

CODE STYLE

S2 - Use indentation properly (R)

Eclipse can help you with this. Source > Correct Indentation, or Source > Format. Use these commands JUST for the code you're adding or fixing. Don't apply to all the class.

S4 - Choose good names for your variables, classes and methods (B)

There is no golden rule here, a good name must express the content of a variable, or the process done in the method, or the set of objects comprised by the class.

S6 - Use SysConfig parameters when possible (don't hardcode) (B)

The only valid hardcoded is with official dictionary System ID's (not GardenWorld ID's). In this case it's recommended to use a java constant with proper name.

S7 - Use support libraries whenever possible, like Env, Util, DB (R)

It's strongly recommended to research the utilities provided by some support classes, like Env, Util, DB, and use the proper utility classes.

REPOSITORY PUSHING

P1 - Always related to a tracker (B)

The only exception to this rule is minor beautify or refactoring, this is: changing comments, changing variable names, improving indentation. Things that don't change the logic of the program can be committed with a simple message indicating the purpose of the change, not related to a tracker.

Whenever changing the logic of a program (even replacing a known method by another known) must be related to a tracker.

P2 - Log message (R)

First line of the log message is the most important – use it like the subject of an e-mail, this is because many log tools just show the first line of the commit message.

Comment (reference the tracker, link, explanation)

Update the tracker with reference to commit revision

P3 - Atomic commit (S)

This is a must to ease the merging on other branches

P6 - Check build process (S)

Specially when changing location of classes, or adding new classes, if you're working in eclipse one-big-project setup you need to check that you didn't break.

In case you did the mistake, hudson server will inform about the breaking within the next hour. Please be aware of the hudson message to fix the issue or revert.

MAINTENANCE

M1 – Big new contributions must have a maintainer (S)

To contribute big things or complete modules at least one committer must agree to become maintainer. Otherwise, please consider to contribute it as extension.

M2 – Dropping from release if maintainer disappear (R)

When maintainer of a module is inactive we need to consider dropping the module from /release and keeping it as extension.

---

Date: December 29 of 2009

Author: Carlos Ruiz – acting as PMC Head