Difference between revisions of "ADempiere Best Practices"

From ADempiere
Jump to: navigation, search
This Wiki is read-only for reference purposes to avoid broken links.
m (→‎Suggested policies for extensions)
m (→‎How to return only ONE persistent object?)
Line 343: Line 343:
 
  MAccount existingAccount = new Query(ctx, I_C_ValidCombination.Table_Name, whereClause, null)
 
  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})
 
  .setParameters(new Object[]{AD_Client_ID, AD_Org_ID, C_AcctSchema_ID, Account_ID})
−
  .'''firstOnly'''();
+
  .firstOnly();
 
</pre>
 
</pre>
  

Revision as of 00:40, 3 January 2009

Contents

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



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



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



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



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



to integrate in trunk it must be complete - documented - testeable


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



it must support oracle and postgres and pl/java


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



it must support java 5 and java 6


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



Bug fixing can be done directly - it needs to be straight


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



Code must be properly indented


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

... but don't re-indent fully working code just because it doesn't look exactly right in your IDE. The code might be indented nicely in someone elses IDE and if we keep reformatting according to our particular IDE we'll end up with a lot of commits but nothing really done. We won't be able to track "real" changes.



proper usage of variable names


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



all comments and variables in english


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



no duplicated code


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



using constants or MSysConfig instead of hardcoded things


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



Suggestions & Questions of Commit Policy


Suggestions (advices) about hiring sponsored developments to ease inclusion in trunk


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



Advices suggested on Jul-2007


License GPLv2


  •  Sponsored development must be released Open Source with the same license from Adempiere (currently GPLv2)


  •  If some development is put in wiki, discussed with community, enhanced with community ideas, etc. 


The best practice can be to release it with the same Adempiere license. 
Very possibly community would not help or opine in a sponsored development that will be released as proprietary. 



Contributor Agreement


  •  All sponsored development must be contributed signing an Adempiere Contributor Agreement (just one agreement by developer) like the one proposed here: Contributor_Agreement 


  •  Initially contributor agreements must be sent to a selected trustee. 


  •  This is helping to ensure the first clause. It's a common practice in Open Source projects after the SCO vs IBM case. 


Peer review


  •  Sponsored development must follow peer review from a committer to be included in trunk - if the developer is a committer, then must have peer review from other committer (same as all contributions in trunk) 


Tools used


  •  In order to ease inclusion in core, the development must follow guidelines from the project - about tool used for integration:

    •  SQL migration scripts 
      • 

    •  if the developer have problems with migration scripts, he must at least provide 2pack package


Must follow Adempiere conventions


  •  Developer must follow Adempiere conventions, i.e. code, directory tree, naming 
    • 

  •  Adempiere conventions are not clearly written, but Adempiere is big enough to get some conventions just looking what currently we have.


Test Cases


  •  It would be good developer provide/document simple test cases for the contribution 


  •  It's a good advice for sponsors when hiring a developer put the condition that deliverables must include documentation and test cases. The developer must know if the money is enough to make such work.


Visibility


  •  Try to keep updated the advance in a wiki page


  •  Simple advice that also can be put as a condition from sponsor to developer in order to maximize the community involvement. 




About branches


  •  Branches are allowed (and in fact they're encouraged) to conduct experiments, to work in specific projects or in tasks, or to coordinate team work.


  •  Private branches are allowed - owner(s) of branch rule the branch (all these rules are suggested in this case, but the owner(s) can expand or shrink these rules)


About releases


  •  Official releases must come from trunk


  •  Official release must be tagged as alpha, beta, release candidate or stable.
    • 

  •  This is subject to the number of errors open, the completeness of functionalities, and other issues related to quality of release.


  •  Unofficial releases are allowed from branches

    •  in this case the release must have the name of the branch
      • 

    •  in this case the release must NOT be tagged as alpha, beta, release candidate or stable, unless community votes and after a review of open bugs, completeness of functionalities, and other issues related to quality of release.







Suggested policies for extensions


Meeting 2008-11-05


According to suggestions received on meeting nov-5-2008 convoked by the more boring ruler.



Generate ID's from centralized dict


Assign specific entity type and prefix rules


It's suggested also to follow entity type and prefix rules for all columns and tables created.



Build on top and not changing kernel


  •  Sometime an extension might replace some code in core but that is usually just a problem in the core rather than changing the definition of an extension


  •  Definition of kernel: common functionalities, like PO, like window management, etc - none related to business rules ...


Work in a branch


  •  It's encouraged to work in an adempiere branch and open for adempiere committers.  But anyways it can be analyzed if extension developer wants to open an own project in sourceforge (or any open source hosting site).


  •  You don't need permissions to experiment in your own branch/project - and add things to core.  If you want something integrated in trunk you must follow rules stated in this document.


  •  Extension developers must have a playground where they can develop without asking too much about permissions - following some recommendations to allow easy integration.


If several extensions are in competence - is suggested to let them compete as extensions, not including any on trunk


Integrating several extensions at the same time can lead to column collision problems.



Follow the recommended extension architecture


  •  Write callouts
    • 

  •  Write ModelValidator
    • 

  •  Don't generate model for your columns (but you can for your new tables), instead of this use the generic getters and setters available in PO.java


License


License must be GPLv2.



No hiding sources


It's encouraged to avoid the SaaS loophole to hide sources (or any other loophole within this license).  Code must be open for everybody in any release.



Certification


If you follow the suggested policies here - you can be certified as adempiere friendly extension - and we can consider including it within the official release.



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:



  1.  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
    • 

  • (pls edit according to SF link findings above)


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.
    • 

  • New features, windows or fields need help text at least in english.


Lawful Vote and Review of Best Practices


For more info please see Project_Charter#Political.



  • Review is now ongoing until 31st December 2008, where it is accepted and voted en bloc by the whole community in January 2009.
    • 

  • It has to be a full turn-out consensus vote first time round from the active common members in order for it to carry the weight of enforceable law.
    • 

  • Subsequent reviews shall be periodic minimally each quarter and not amended adhoc prior to review date.
    • 

  • Next review date should be not earlier than March 2009.


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).




									

						Retrieved from "http://www.adempiere.com/index.php5?title=ADempiere_Best_Practices&oldid=3463"