Skip to content

Conversation

@sudhir-erpharbor
Copy link
Contributor

This module depends on base_exception (server-tools) module.
Please review and give feedback.

@pedrobaeza pedrobaeza added this to the 12.0 milestone Nov 23, 2018
@OCA-git-bot OCA-git-bot mentioned this pull request Nov 23, 2018
25 tasks
Copy link
Member

@nikul-serpentcs nikul-serpentcs left a comment

Choose a reason for hiding this comment

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

Improve Code

Copy link

@JayVora-SerpentCS JayVora-SerpentCS left a comment

Choose a reason for hiding this comment

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

@sudhir-erpharbor
Copy link
Contributor Author

Any more improvements / suggestions?
@pedrobaeza @JayVora-SerpentCS @nikul-serpentcs

@Nikul-Chaudhary
Copy link
Member

@sudhir-erpharbor IMO Add oca_dependencies file,

server-tools https://github.com/osipaas/server-tools 12.0-mig-base_exception

Because Travis is fail here, depends module not available

@Nikul-Chaudhary
Copy link
Member

@sudhir-erpharbor squash commits

@pedrobaeza could you please review code?

Copy link
Contributor

@florian-dacosta florian-dacosta left a comment

Choose a reason for hiding this comment

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

Hello,
The base_exception module has been refactored in version 10 (OCA/server-tools#1586)
The same is beeing done in version 12 and the refactore should be merged soon : OCA/server-tools#1589

This changes the way to implement sub modules (purchase_exception, sale_exception, etc...)
So this PR should be adapted to this new version. You can find an example with sale_exception module here : OCA/sale-workflow#860

@sudhir-erpharbor Can you adapt the module in this PR?
If necessary, I can eventually propose a PR with the module adapted to new version.

@bealdav
Copy link
Member

bealdav commented Jun 5, 2019

see the fix here to be compatible with new exception sudhir-erpharbor#1

Thanks to review

@bealdav
Copy link
Member

bealdav commented Jun 10, 2019

It seems ready to merge. Could you approve @JayVora-SerpentCS and @florian-dacosta

Copy link
Contributor

@florian-dacosta florian-dacosta left a comment

Choose a reason for hiding this comment

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

Just a small comment.
No really blocking but it would be appreciated to do it now.

@florian-dacosta
Copy link
Contributor

Thanks!
👍

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@pedrobaeza pedrobaeza merged commit 221e481 into OCA:12.0 Jun 10, 2019
@sudhir-erpharbor sudhir-erpharbor deleted the 12.0-purchase_exception branch June 11, 2019 09:52
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.