-
-
Notifications
You must be signed in to change notification settings - Fork 360
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
[13.0] Add delivery_postlogistics_dangerous_goods #367
[13.0] Add delivery_postlogistics_dangerous_goods #367
Conversation
0278821
to
0758137
Compare
0758137
to
ce84dd1
Compare
], | ||
"website": "https://github.com/OCA/delivery-carrier", | ||
"installable": True, | ||
"auto_install": False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto_install
False is the default, so not really needed, here.
But does it not make sense to have it set to True
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mmequignon ping
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I don't know in what case we should enable auto-install or not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With auto_install
set to True
your module will be automatically installed when both its dependencies are installed. Which in my opinion makes sense in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense to set it to True
as this module doesn't break the base PostLogistics module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG, one comment by @TDu needs answer
], | ||
"website": "https://github.com/OCA/delivery-carrier", | ||
"installable": True, | ||
"auto_install": False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mmequignon ping
This PR has the |
/ocabot merge nobump |
What a great day to merge this nice PR. Let's do it! |
@dreispt your merge command was aborted due to failed check(s), which you can inspect on this commit of 13.0-ocabot-merge-pr-367-by-dreispt-bump-nobump. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
ce84dd1
to
1565b8a
Compare
1565b8a
to
6c61162
Compare
@mmequignon Can you please fix the CI ? |
6c61162
to
2e5d1f0
Compare
/ocabot merge nobump |
Hey, thanks for contributing! Proceeding to merge this for you. |
@sebalix your merge command was aborted due to failed check(s), which you can inspect on this commit of 13.0-ocabot-merge-pr-367-by-sebalix-bump-nobump. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
/ocabot merge nobump |
This PR looks fantastic, let's merge it! |
dda5608
to
939ed46
Compare
939ed46
to
7eabbca
Compare
Let's try again... |
What a great day to merge this nice PR. Let's do it! |
@sebalix your merge command was aborted due to failed check(s), which you can inspect on this commit of 13.0-ocabot-merge-pr-367-by-sebalix-bump-nobump. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
Is it intended to have that kind of output? It seems too verbose and not giving significant informations. The label binary gives a lot of noise. Travis is still unhappy. https://app.travis-ci.com/github/OCA/delivery-carrier/jobs/543251740#L1416 |
Hi @mmequignon we faced this issue before and a workaround was to mute vcr logger for the case involving the big response; I tried it here, seems to work |
/ocabot merge nobump |
Hey, thanks for contributing! Proceeding to merge this for you. |
@dreispt thanks Daniel, but I think there's some action to do on my side before trying. |
@dreispt your merge command was aborted due to failed check(s), which you can inspect on this commit of 13.0-ocabot-merge-pr-367-by-dreispt-bump-nobump. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
@mmequignon No news on this ? |
@mmequignon ping |
we keep only v14 OCA/wms#334 |
This modules overrides
Postlogistics Shipping
to declare dangerous goodsduring postlogistics label generation.
Based on:
Supersedes: