openerp-expert-framework team mailing list archive

[Olivier Dony <odo@xxxxxxxxxxx>]

On 04/16/2012 07:37 PM, Valentin LAB wrote:
> 
> Is there any coding convention enforced on the OpenERP Code ?

As Lionel (Numérigraphe) said, we try to enforce common-sense best practices
when working on new developments, refactoring existing code, and every
opportunity we have to improve code.

However, we prefer to avoid changing unimportant things like whitespace,
line-wrapping etc when we don't have any other reason to touch the code.
In general it has little value compared to the cost of decreasing the
visibility of the main goal of the patch, creating extraneous merge conflicts,
and even causing unexpected regressions.

In rare cases it may make sense to do a quick mass-cleanup, if and when the
timing is convenient.


> I'm thinking about:
> - code pure aesthetic as the PEP08, or PEP257
>     http://www.python.org/dev/peps/pep-0008/
>     http://www.python.org/dev/peps/pep-0257/
>     (ie: trailing whitespace, 80 col ...)

Those PEPs are very good but we don't enforce them to the letter, common sense
always prevails [1]. As an example, I will never hold it against any developer
if they don't strictly wrap their line at 80 chars, because we don't have good
reasons to enforce it. When was the last time you _really_ needed 3 files
side-by-side on the same screen? OTOH that wrapping length flexibility usually
improves readability (e.g. 90 cols are fine if it avoids wrapping at all)


> - best practice as checked by pylint
>     (size of variables, number of method, ...)
> - code coverage checks
> - code complexity checks

For these kinds of improvements we definitely welcome patches, such as
refactoring of over-sized methods, dead code removal, etc. But one has to keep
in mind that there's always a risk, these tools only give hints...


I hope this gives you a general idea of what to do when something is bugging
you in the source code... ;-)


[1] PEP8 starts by telling you when to break those rules that it has not stated
yet.. there's a reason to that :-)