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

Revision as of 21:30, 18 December 2008

1 Overview
2 Goal
3 Team
4 Most boring ruler
5 Development Methodology
- 5.1 Sugesstion & Questions Development Methodology
6 SVN Commit Policy
- 6.1 Reference
7 Rules discussed in forums
8 Coding Standards
- 8.1 Sugesstion & Questions Coding Standards
9 Coding Style
10 Testing Policy
- 10.1 Test Units
11 Documentation Policy
12 Mentor Policy
13 PMC and Commit Committee Policy
14 PMC and CC membership Policy

Overview

Goal

Team

Teo Sarca Arhipac : Libero stabilization

Cristina Ghita Arhipac : Libero stabilization

Trifon D3Soft

Victor Perez e-Evolution

Dominik Zajac BayCIX GmbH

Most boring ruler

Carlos Ruiz - just read links below

Development Methodology

Sugesstion & Questions Development Methodology

Eclipse Process Framework Project (EPF) Demo

SVN Commit Policy

FIRST DRAFT PLEASE ADD TO AS YOU SEE FIT... The following guidelines describe the best practice when committing code in the Adempiere repository.

All commits should be atomic: that is they are the complete code patch that addresses a Bug Report [BR] or Feature Request [FR] in sourceforge. If the contribution is unusually large it may be committed in small parts but then commit should be distinct functional parts.

Commit Early in YOUR day: Only commit if you are around to support or even revert in case of a problems.

All commits related to BR or FR: with consensus on the functional and technical change how the change will be accomplished (in general).

Update before commit: Do an SVN update and ensure all still compiles locally before you commit.

Reference the BR/FR in the commit comments: include the full url to the sourceforge BR/FR, but also include a short, but descriptive, comment that does not require the reader to go to the BR/FR in sourceforge unless they require details.

Example:

[ 2354040 ] Implementation Replication Mode, Type, Event

http://sourceforge.net/tracker/index.php?func=detail&aid=2354040&group_id=176962&atid=879335

Enhance replication to allow export and import of documents and then execute the document status action.

Syncronise & Build: after committing sync again with the repository and confirm all still builds without errors.

On successful commit Update the BR/FR: include the SVN revision reference and set the status & resolution fields as appropriate.

Reference

SVN Best Practices

Rules discussed in forums

to have autonomy you need a high degree of responsibility

https://sourceforge.net/forum/message.php?msg_id=4749708

Rules suggested on Nov-2007 to reach TRUNK

https://sourceforge.net/forum/forum.php?thread_id=1881039&forum_id=611167

Proof of stability

This should be base on community feedback on binary release.
Optional: Production site, unit and functional test case.

Migration Process COMPLETE and documented

For new module that replace or enhance existing functionality, it is very important to have the migration process documented.

Sample Data

There should be some sample data available.

Technical Documentation

I think it would be good to have some technical documentation (trying not to be very extensive to avoid suffocating innovation)
What if somebody contribute a module and then disappear, who is going to maintain it?
We would need to do reversal engineering to understand the contribution.
Maybe just a short explanation about what is being contributed.

Maintainer

Must have dedicated maintainer ( individual or company ).
Must follow existing Adempiere Developer's criteria.

Branding and copyright issue

No vendor branding, must prove to be original work.

Migration script

Support all supported database, i.e oracle and postgresql and pl/java

More rules in forums on Dec-2008

Trackers must be descriptive

http://sourceforge.net/forum/forum.php?thread_id=2681108&forum_id=610548

One-line trackers are not respectful of other eyeballs reviewing.

Trackers must be descriptive.

If you're fixing a bug - the best is if you can explain steps to reproduce the bug in GardenWorld or System. If it's not easily reproducible in GardenWorld, then you can drop some lines explaining what's the issue, in which scenarios the issue appears and the proposed solution (if any).

This is not just about code - it's about communicating - it's about other eyeballs helping with the review (the most important asset given that we don't have unit testings).

If you're reporting a feature request - also try to be descriptive of the scenario.

If you're implementing a feature request - then you must explain how to use the new functionality - how to enable it - how to disable it (if possible) - how to use it (better in a wiki page) - etc.

It's not good just to say - feature implemented - and put everybody here to read code to know HOW it was implemented.

Don't drop not-added-by-you things

http://sourceforge.net/forum/forum.php?thread_id=2667992&forum_id=610548

Don't drop "not-added-by-you" things (db elements or functionalities).

If it was added by you in previous versions - then it must not be removed.

If the thing is wrong - or functionality is really broken - then please first ask in forums before considering the drop.

Always review collaterals

http://sourceforge.net/forum/forum.php?thread_id=2668000&forum_id=610548

Every time you touch one line of code or application dictionary please review the dependences (collaterals). Eclipse helps a lot pointing to the name of the method/variable + right click + references -> workspace.

For other code (like callouts, processes, etc) sometimes you need to check the dictionary to see where is it called (I normally review Adempiere_pg.dmp).

not to commit a change if you only have solved it for one database

http://sourceforge.net/forum/forum.php?thread_id=2668000&forum_id=610548

Just one version of ModelGenerator

http://sourceforge.net/forum/forum.php?thread_id=2663645&forum_id=610548

There must be just ONE AND OFFICIAL version of ModelGenerator. Private versions of ModelGenerator must be avoided.

New features entering trunk must be tested first in an experimental / project / task branch

http://sourceforge.net/forum/message.php?msg_id=5828131

Suggestions & Questions of Commit Policy

Coding Standards

Sugesstion & Questions Coding Standards

Coding Style

See Eclipse Code Formatter Profile .

1. Use DB.getSQLValueEx instead of DB.getSQLValue. It's much better to throw an exception if error. If you use DB.getSQLValue, will not make the difference between "did not find any value" and was an exceptional SQL.

2. Do not use the conversion from Integer to int, Double to double and so on, because from java5 is doing autoboxing.

3. Allways use bracelets {}.

4. Don't use transaction names if is not necessary. Better put null.

Known issues:

At present eclipse formatter is not supporting fluent interfaces (see eclipse bug #196001)

How to use Adempiere Query class?

How to return only ONE persistent object?

StringBuilder whereClause = new StringBuilder();
		whereClause.append("AD_Client_ID=?");          // #1
		whereClause.append(" AND AD_Org_ID=?");        // #2
		whereClause.append(" AND C_AcctSchema_ID=?");  // #3
		whereClause.append(" AND Account_ID=?");       // #4

 MAccount existingAccount = new Query(ctx, I_C_ValidCombination.Table_Name, whereClause.toString(), null)
 		.setParameters(new Object[]{AD_Client_ID, AD_Org_ID, C_AcctSchema_ID, Account_ID})
 		.first();

If you know that your query should return ONLY one result, then you can assert this, and use firstOnly method instead of first method:

 MAccount existingAccount = new Query(ctx, I_C_ValidCombination.Table_Name, whereClause, null)
 		.setParameters(new Object[]{AD_Client_ID, AD_Org_ID, C_AcctSchema_ID, Account_ID})
 		.'''firstOnly'''();

How to return ARRAY of objects?

public static MAchievement[] getOfMeasure (Properties ctx, int PA_Measure_ID)
{
	final String whereClause = COLUMNNAME_PA_Measure_ID+"=?"; 
	List<MAchievement> list = new Query(ctx, I_PA_Achievement.Table_Name, whereClause, null)
		.setParameters(new Object[]{PA_Measure_ID})
		.setOrderBy(COLUMNNAME_SeqNo+", "+COLUMNNAME_DateDoc)
		.list();
	return list.toArray(new MAchievement[list.size()]);
}

How to return ARRAY of objects and process elements?

public static MAchievement[] getOfMeasure (Properties ctx, int PA_Measure_ID)
{
	String whereClause = COLUMNNAME_PA_Measure_ID+"=? AND "+COLUMNNAME_IsAchieved+"=?"; 
	List<MAchievement> list = new Query(ctx, MAchievement.Table_Name, whereClause, null)
		.setParameters(new Object[]{PA_Measure_ID, true})
		.setOrderBy(COLUMNNAME_SeqNo+", "+COLUMNNAME_DateDoc)
		.list();
	for(MAchievement achievement : list)
	{
	  s_log.fine(" - " + achievement);
	  // do some processing here
	}
	return list.toArray(new MAchievement[list.size()]);
}

How to return one member of an object?

	public static int getWindow_ID(String windowName)
        {
	    int retValue = 0;
 	    String whereClause = COLUMNNAME_Name+"=?";
	    MWindow win = new Query(Env.getCtx(), MWindow.Table_Name, whereClause, null)
		  .setParameters(new Object[]{windowName})
 		  .first();	
 	     return = win.getAD_Window_ID(); 
        }

How to pass Timestamp parameter?

Timestamp dateAcct = ...;
String trxName = ...;

StringBuilder whereClause = new StringBuilder();
		whereClause.append("C_CashBook_ID=?");				//	#1
		whereClause.append(" AND TRUNC(StatementDate)=?");		//	#2
		whereClause.append(" AND Processed=?");				//	#3
		
MCash retValue = new Query(ctx, MCash.Table_Name, whereClause.toString(), trxName)
	.setParameters(new Object[]{C_CashBook_ID, TimeUtil.getDay(dateAcct), true})
	.first()
;

How to use Query class with complex where clause: EXISTS?

StringBuilder whereClause = new StringBuilder();
	whereClause.append("C_Cash.AD_Org_ID=?");			// #1
	whereClause.append(" AND TRUNC(C_Cash.StatementDate)=?");	// #2
	whereClause.append(" AND C_Cash.Processed='N'");		
	whereClause.append(" AND EXISTS (SELECT * FROM C_CashBook cb");
	whereClause.append(" WHERE C_Cash.C_CashBook_ID=cb.C_CashBook_ID AND cb.AD_Org_ID=C_Cash.AD_Org_ID");
	whereClause.append(" AND cb.C_Currency_ID=?)");			// #3
MCash retValue = new Query(ctx, MCash.Table_Name, whereClause, trxName)
	.setParameters(new Object[]{AD_Org_ID,TimeUtil.getDay(dateAcct),C_Currency_ID})
	.first()
;

References

EE Best Practices

Precise Java

Sugesstion & Questions Coding Style

When on a single thread class, StringBuilder should be used for String concatenation. If the class is not single threaded, then, StringBuffer should be used for String concatenation.

Testing Policy

Submit your changes to local testing or a peer review before committing unless you are approved by 1st level committer
Publish your test results

Test Units

... (test unit contributions needed)

Documentation Policy

Document your sourcecode changes in this wiki under appropriate topic.
Solicit help from others if you cannot document well. Or just start a stub and allow others to expand it.
Intentionally hiding information may get your contribution categorised as proprietary and not fit for admission into trunk.

Mentor Policy

All committers should also seek a mentor or coach among the senior committers already mentioned in the Commit Committee.
Mentees (who are under Mentors) have to assist in providing review and other administrative assistance (this is due to resources constraints at the moment to manage the team).

PMC and Commit Committee Policy

(to be discussed in talk page if it is not in agreement as this is only a quick expedient suggestion)

PMC (Project Management Committee) and CC (Commit Committee) may overrule decisions based on technical and urgent situations. In case of conflict with the general community, the PMC may hold a meeting and only PMC members can vote on the outcome.
Community members must abide by rulings of the PMC and CC.
PMC may overrule a feature acceptance or stable/non-stable status of any feature.
CC may overrule a trunk (stable) commit decision. It may ask the committer to revert successfully failing which it can suspend that committer's right for a short period.
Repeat offenders may be subjected to community decision and voting for permanent suspension.

PMC and CC membership Policy

PMC and CC members may be suggested and voted in based on their merit and contributions.
There should not be a single negative vote from PMC and CC members to a nominee (to avoid poison and disruption to the ongoing work).
If there is a negative vote, the nominator/nominee should address the negativity underlying reasons and try for a nomination again at a later date.
Thus though rulings within PMC/CC may be democratic but joining it is not. It has to be meritocracy in order to join the PMC and CC.

@@ Line 322: / Line 322: @@
 *Intentionally hiding information may get your contribution categorised as proprietary and not fit for admission into trunk.
+=Mentor Policy=
+*All committers should also seek a mentor or coach among the senior committers already mentioned in the [[Commit Committee]].
+*Mentees (who are under Mentors) have to assist in providing review and other administrative assistance (this is due to resources constraints at the moment to manage the team).
+=PMC and Commit Committee Policy=
+(to be discussed in [[Talk:ADempiere_Best_Practices|talk]] page if it is not in agreement as this is only a quick expedient suggestion)
+*PMC ([[Project Management Committee]]) and CC (Commit Committee) may overrule decisions based on technical and urgent situations. In case of conflict with the general community, the PMC may hold a meeting and only PMC members can vote on the outcome.
+*Community members must abide by rulings of the PMC and CC.
+*PMC may overrule a feature acceptance or stable/non-stable status of any feature.
+*CC may overrule a trunk (stable) commit decision. It may ask the committer to revert successfully failing which it can suspend that committer's right for a short period.
+*Repeat offenders may be subjected to community decision and voting for permanent suspension.
+=PMC and CC membership Policy=
+*PMC and CC members may be suggested and voted in based on their merit and contributions.
+*There should not be a single negative vote from PMC and CC members to a nominee (to avoid poison and disruption to the ongoing work).
+*If there is a negative vote, the nominator/nominee should address the negativity underlying reasons and try for a nomination again at a later date.
+*Thus though rulings within PMC/CC may be democratic but joining it is not. It has to be meritocracy in order to join the PMC and CC.
 [[Category:Community]]
 [[Category:Documentation]]

Difference between revisions of "ADempiere Best Practices"