Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ADD] l10n_be_iso20022_pain #10

Merged
merged 1 commit into from
Mar 25, 2015
Merged

Conversation

sbidoul
Copy link
Member

@sbidoul sbidoul commented Dec 14, 2014

This PR depends on OCA/bank-payment#80

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) when pulling 1a4a11f on acsone:8.0-l10n_be_sepa-sbi into ed7088d on OCA:8.0.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.81%) when pulling 026bbd6 on acsone:8.0-l10n_be_sepa-sbi into ed7088d on OCA:8.0.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.81%) when pulling f6dcc1f on acsone:8.0-l10n_be_sepa-sbi into ed7088d on OCA:8.0.

@lmignon
Copy link
Sponsor Contributor

lmignon commented Dec 15, 2014

👍 LGTM (code review, no tests)

{
'name': 'SEPA Support for Belgium',
'version': '1.0',
'author': 'ACSONE SA/NV',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not add OCA as author ?

@luc-demeyer
Copy link
Contributor

The name of the module is confusing and incorrect.
SEPA is the Single European Payment Area, hence payments in euro, whereas the pain protocol allows both SEPA and non-SEPA payments (cr. account_pain module which supports both).

A more correct name would be l10n_be_pain.(or l10n_be_iso20022_pain).

I think that this module (once renamed) is also the correct place to add the logic for the InitgPty.Id.OrgId.Othr.Id and InitgPty.Id.OrgId.Othr.Issr fields (cf. febelfin specs).

I can make a PR for this but prefer to do this on a module with a correct naming.

@sbidoul sbidoul changed the title [ADD] l10n_be_sepa [ADD] l10n_be_iso20022_pain Mar 21, 2015
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.81%) to 63.11% when pulling 11bbb66 on acsone:8.0-l10n_be_sepa-sbi into 5fc3106 on OCA:8.0.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.81%) to 63.11% when pulling 11bbb66 on acsone:8.0-l10n_be_sepa-sbi into 5fc3106 on OCA:8.0.

@sbidoul
Copy link
Member Author

sbidoul commented Mar 21, 2015

@adrienpeiffer I adapted the README.
@luc-demeyer I renamed the module.

Regarding the InitgPty.Id.OrgId.Othr I'll do something when we have reached a conclusion in OCA/bank-payment#131 and/or OCA/bank-payment#112

@adrienpeiffer
Copy link

👍 Thanks !

@lmignon
Copy link
Sponsor Contributor

lmignon commented Mar 25, 2015

👍 (Code review)

sbidoul added a commit that referenced this pull request Mar 25, 2015
@sbidoul sbidoul merged commit 221903d into OCA:8.0 Mar 25, 2015
@sbidoul sbidoul deleted the 8.0-l10n_be_sepa-sbi branch March 25, 2015 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants