migrate payment addons to this repo. - Githubissues

OCA / l10n-china

Chinese localization of Odoo

GNU Affero General Public License v3.0

32 stars 46 forks source link

migrate payment addons to this repo. #14

Closed jeffery9 closed 7 years ago

oca-clabot commented 8 years ago

Hey @jeffery9, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet. You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla Here is a list of the users:

@leangjia (login unknown in OCA database)

Appreciation of efforts, OCA CLAbot

jeffery9 commented 8 years ago

update, @leangjia have signed CLA.

leangjia commented 8 years ago

@oca-clabot I have signed ICLA. @elicoidal
@jeffery9

elicoidal commented 8 years ago

Hi @leangjia @jeffery9 Thanks for the contribution and sorry for the delay. Let me introduce to you some basic information about contribution in OCA and I will review other points in more comments inside the code. A more technical review will be necessary as well (I am not technical myself). In general, please follow the principles:

Contribution guidelines: https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md
Please follow the OCA README template: https://github.com/OCA/maintainer-tools/blob/master/template/module/README.rst
We recommend to include a screenshot as well to help new comers (nice to have)
Please use simple headers in all py files following OCA templates https://github.com/OCA/maintainer-tools/blob/master/template/module/__openerp__.py#L3
Please sort all imports in alphabetical order following: https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md#imports

elicoidal commented 8 years ago

Please note as well that there is in this repo another 2 PR (#6 and #11) for wc and alipay connectors in v8. Please review them (comments are welcome) and try to merge your current work inside the existing PR. allpay and tenpay connector should be proposed as 2 different PR (we usually have one PR per module to ease the review and feedback).

elicoidal commented 8 years ago

Note as well that your code should contain unit tests

elicoidal commented 8 years ago

Thanks for the contribution! This is an impressive work for a initial version. I hope you understand my comments but do not hesitate to contact me if necessary. Once the initial PR have been fixed, we will make a more technical review and then work on the README content.

leangjia commented 8 years ago

@elicoidal Thanks for your help, I appreciate that.

1.Both of payment_weixin and tenpay belong to Tencent company in local. 2.Allpay and alipay are different. Allpay is a kinda Taiwan payment tool. Alipay belongs to Alibaba company in China continent.

BR & THANK YOU

elicoidal commented 8 years ago

@leangjia I understand: as said, alipay and wechat have already proposed PR in this repo. Can you check them and propose to merge your improvements in those PR.

jeffery9 commented 8 years ago

this PR should be break into several pieces.

elicoidal commented 8 years ago

I close this PR as it is replaced by #16 and #17