-
Notifications
You must be signed in to change notification settings - Fork 23.3k
Reviewing
As an Odoo employee, how to review external (and internal) pull requests:
- List of PR from the community:
- See Stability Guidelines
- Not breaking anything is not enough, a patch must fix a bug
- Some customisations or modules (officials or not) may rely on a behaviour, changing it will break these modules
- No new feature
- No change of field definition
- No new SQL constraint
- No change of
_order
- No change of signature on public methods
- No hook method (helping to modify standard behaviour)
- No code cleaning or cosmetic changes
- No change of translatable content (labels, templates, messages,...)
- The older the version is, the stricter you must be
- The older a version is, the bigger is the risk of somebody relying on a weird behaviour
- New features and change of behaviour are accepted
- No need to worry about translations (no changes in
.po
or.pot
files) - Use the Security Checklist
- See Commit Message Guidelines
- In 90% of the time, the commit message does not respect our guidelines, don't be lazy, change it!
- Explain the why, not what
- See sign the CLA
- The verification is based on the last commit email address
- Add
.patch
at the end of the URL to find it easily
- Add
- The verification is based on the source branch, not target, make sure it is rebased
- See translation guidelines
- Never change
.po
files- to be done on Transifex
- a few exceptions are allowed like localisations or branches/languages not on Transifex
- Adapt the
.pot
if adding or removing terms (in stable) - Modifying existing term will make it untranslated for many people
The code is probably on a new remote, to test it (and adapt it if needed) you have to add the remote and branch.
- To help you, add the get-br alias to your
~/.gitconfig
$ git get-br remote-name:branch-name
-
$ git commit --amend
modify the commit message -
$ git rebase odoo/12.0
rebase branch (and squash?) -
$ git push -f
push force (or consider--force-with-lease
) on target branch- in most cases you have the rights to do it (allowed by default)
- if not you have to create a branch on
odoo-dev
with the amended code, create pr,...
@robodoo r+
- Remove the added remote and branch
$ git branch -D branch-name
$ git remote remove remote-name
- or, if you are lazy, use the rem-br alias
$ git rem-br remote-name:branch-name
Last but not least, be nice to people!
Somebody took the time to create a contribution to Odoo. They may not be professional developers nor they know our specific guidelines. They may not know all the corner cases or how to use git properly.
Most of the time the contributions will not be good enough on the first try or may even be rejected. Explain why you can not accept a patch.
- Contributing Guidelines
- Google's guide on How to do a code review
Website | Online Demo | Community | eLearning | Help | Scale-Up business game
Boost Sales: CRM | Point of Sale | Quote Builder | Mass Mailing | Survey | Events
Build Websites: CMS | eCommerce | Blogs | Forum | Get a Free Website
Run Operations: Projects | Billing | Accounting | Inventory | Manufacturing | Procurements
Delight Employees: Employees | Recruit | Expenses | Appraisals | Fleet