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

[REF] TEST CI #217

Merged
merged 10 commits into from
Jan 24, 2022
Merged

[REF] TEST CI #217

merged 10 commits into from
Jan 24, 2022

Conversation

legalsylvain
Copy link
Member

No description provided.

@legalsylvain
Copy link
Member Author

/ocabot merge patch
;-)

@github-grap-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 12.0-ocabot-merge-pr-217-by-legalsylvain-bump-patch, awaiting test results.

github-grap-bot added a commit that referenced this pull request Jan 24, 2022
Signed-off-by legalsylvain
@github-grap-bot
Copy link
Contributor

@legalsylvain your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-217-by-legalsylvain-bump-patch.

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.

@legalsylvain
Copy link
Member Author

/ocabot merge patch

@github-grap-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 12.0-ocabot-merge-pr-217-by-legalsylvain-bump-patch, awaiting test results.

github-grap-bot added a commit that referenced this pull request Jan 24, 2022
Signed-off-by legalsylvain
@github-grap-bot
Copy link
Contributor

@legalsylvain your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-217-by-legalsylvain-bump-patch.

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.

@legalsylvain
Copy link
Member Author

/ocabot merge patch

@github-grap-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 12.0-ocabot-merge-pr-217-by-legalsylvain-bump-patch, awaiting test results.

github-grap-bot added a commit that referenced this pull request Jan 24, 2022
Signed-off-by legalsylvain
@github-grap-bot
Copy link
Contributor

@legalsylvain your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-217-by-legalsylvain-bump-patch.

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.


# https://github.com/OCA/purchase-workflow/pull/1185
git+https://github.com/legalsylvain/purchase-workflow@12.0-purchase_xxx_discount-apply-default-value#subdirectory=setup/purchase_discount
git+https://github.com/legalsylvain/purchase-workflow@12.0-purchase_xxx_discount-apply-default-value#subdirectory=setup/purchase_triple_discount
Copy link

Choose a reason for hiding this comment

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

Note this works too and helps removing a comment. For PRs you control this might be useful. For PR you don't control, the drawback is less stability in case the author rebases or something.

Suggested change
git+https://github.com/legalsylvain/purchase-workflow@12.0-purchase_xxx_discount-apply-default-value#subdirectory=setup/purchase_triple_discount
git+https://github.com/OCA/purchase-workflow@refs/pull/1185/head#subdirectory=setup/purchase_triple_discount

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks a lot for taking time to take a look on my CI problems !

For PR I don't control (quite theoritical case, it doesn't occure in fact), if I understand correctly git / github, comparing :

A) git+https://github.com/acsone/purchase-workflow@12.0-BRANCH-NAME#subdirectory=setup/MODULE

B) git+https://github.com/OCA/purchase-workflow@refs/pull/xxx/head#subdirectory=setup/MODULE

if acsone delete the branch 12.0-BRANCH-NAME, B should still work (event if the PR is closed) but A will fail, right ?

Second question, I have a lot of dependency in that repo, of my incubator repo. It seems it take a while to fetch all the code on the github CI with such requirements :

git+https://github.com/grap/grap-odoo-incubator@12.0#subdirectory=setup/name_search_reset_res_partner
git+https://github.com/grap/grap-odoo-incubator@12.0#subdirectory=setup/purchase_package_qty
git+https://github.com/grap/grap-odoo-incubator@12.0#subdirectory=setup/mobile_kiosk_abstract

do you know if there are a syntax to do something like that, to fetch many modules ?

git+https://github.com/grap/grap-odoo-incubator@12.0#subdirectory=setup/name_search_reset_res_partner,setup/purchase_package_qty,setup/mobile_kiosk_abstract

Copy link

Choose a reason for hiding this comment

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

if acsone delete the branch 12.0-BRANCH-NAME, B should still work (event if the PR is closed) but A will fail, right ?

Yes that is correct. Although with B you are still at risk of acsone changing the PR with commits you don't want.

Second question, I have a lot of dependency in that repo, of my incubator repo. It seems it take a while to fetch all the code on the github CI with such requirements :

Yes I'm aware of that. I had an optimization that broke with v15 and that I need to re-do.

@codecov
Copy link

codecov bot commented Jan 24, 2022

Codecov Report

Merging #217 (a2ec0d8) into 12.0 (52346b4) will increase coverage by 2.80%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##             12.0     #217      +/-   ##
==========================================
+ Coverage   72.79%   75.60%   +2.80%     
==========================================
  Files          94      117      +23     
  Lines         794      910     +116     
  Branches        0      169     +169     
==========================================
+ Hits          578      688     +110     
- Misses        216      217       +1     
- Partials        0        5       +5     
Impacted Files Coverage Δ
...se_product_mass_addition/models/product_product.py 26.92% <ø> (ø)
grap_change_email/wizards/account_invoice_send.py 57.14% <0.00%> (ø)
...e_views_account/models/account_account_template.py 62.50% <100.00%> (ø)
...hange_views_account/models/account_account_type.py 62.50% <100.00%> (ø)
grap_qweb_report/models/report_custom_message.py 100.00% <100.00%> (ø)
..._qweb_report/models/report_custom_message_mixin.py 85.71% <100.00%> (-9.53%) ⬇️
grap_qweb_report/tests/test_module.py 95.23% <100.00%> (ø)
grap_qweb_report/models/res_company.py 66.66% <0.00%> (-16.67%) ⬇️
grap_change_views_pos/models/product_template.py 66.66% <0.00%> (-16.67%) ⬇️
grap_change_default/models/product_template.py 55.55% <0.00%> (-11.12%) ⬇️
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fede17c...a2ec0d8. Read the comment docs.

@legalsylvain
Copy link
Member Author

/ocabot merge patch

@github-grap-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 12.0-ocabot-merge-pr-217-by-legalsylvain-bump-patch, awaiting test results.

@github-grap-bot
Copy link
Contributor

Congratulations, your PR was merged at 4020219. Thanks a lot for contributing to grap. ❤️

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

3 participants