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

[16][ADD] rma_category #458

Merged
merged 4 commits into from
Oct 27, 2023

Conversation

florian-dacosta
Copy link
Contributor

Add the possibility to add one or more categories on RMA

@codecov-commenter
Copy link

codecov-commenter commented Sep 29, 2023

Codecov Report

Merging #458 (8bcda1c) into 16.0 (282987b) will increase coverage by 0.05%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             16.0     #458      +/-   ##
==========================================
+ Coverage   80.98%   81.04%   +0.05%     
==========================================
  Files         124      128       +4     
  Lines        4807     4822      +15     
  Branches      779      779              
==========================================
+ Hits         3893     3908      +15     
  Misses        701      701              
  Partials      213      213              
Files Coverage Δ
rma_category/__init__.py 100.00% <100.00%> (ø)
rma_category/models/__init__.py 100.00% <100.00%> (ø)
rma_category/models/rma_category.py 100.00% <100.00%> (ø)
rma_category/models/rma_order_line.py 100.00% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

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

Copy link
Contributor

@AaronHForgeFlow AaronHForgeFlow left a comment

Choose a reason for hiding this comment

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

LGTM, I would appreciate a test, but as long as there is no logic we can skip that.

rma_category/README.rst Outdated Show resolved Hide resolved
@florian-dacosta
Copy link
Contributor Author

LGTM, I would appreciate a test, but as long as there is no logic we can skip that.

Indeed, there are not logic at all, just a way to classify the RMA with a M2O. I would not know which test I could do for now !

Copy link
Contributor

@AaronHForgeFlow AaronHForgeFlow left a comment

Choose a reason for hiding this comment

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

LGTM including the fix in RMA view. Can you please put the RMA change in a separate commit? Also, the readme won't be created automatically by the content in the readme folder, so I am afraid you have to do it :'(

@florian-dacosta
Copy link
Contributor Author

LGTM including the fix in RMA view. Can you please put the RMA change in a separate commit? Also, the readme won't be created automatically by the content in the readme folder, so I am afraid you have to do it :'(

I did a separate commit.
About the README, I made the change already 2 days ago.

Copy link
Contributor

@AaronHForgeFlow AaronHForgeFlow left a comment

Choose a reason for hiding this comment

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

Thank you @florian-dacosta

@florian-dacosta
Copy link
Contributor Author

Well, pre-commit fails suddenly, I have no idea why...

@AaronHForgeFlow
Copy link
Contributor

I will check that, perhaps we have to update pre-commit version

@AaronHForgeFlow
Copy link
Contributor

It seems related to change in python version in github actions:

OCA/oca-addons-repo-template#214

@AaronHForgeFlow
Copy link
Contributor

@florian-dacosta if you rebase it should be green now.

@florian-dacosta
Copy link
Contributor Author

Great, thanks !

@AaronHForgeFlow AaronHForgeFlow merged commit a03b279 into ForgeFlow:16.0 Oct 27, 2023
4 checks passed
@florian-dacosta florian-dacosta deleted the 16.0-mig-rma_category branch April 17, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants