Skip to content

Conversation

@victoralmau
Copy link
Member

@victoralmau victoralmau commented Dec 22, 2025

Add _get_location_final() method to be able to use it in other modules

Please @pedrobaeza and @carlos-lopez-tecnativa can you review it?

@Tecnativa TT57139

@OCA-git-bot
Copy link
Contributor

Hi @pedrobaeza, @chienandalu,
some modules you are maintaining are being modified, check this out!

@pedrobaeza pedrobaeza added this to the 18.0 milestone Dec 22, 2025
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Shouldn't this be called _get_final_location_dest?

@victoralmau victoralmau force-pushed the 18.0-imp-rma-location_final branch from 9a51c91 to b2e7f0e Compare December 22, 2025 10:41
@victoralmau victoralmau changed the title [18.0][IMP] rma: Add _get_location_final() method to be able to use it in other modules [18.0][IMP] rma: Add _get_final_location_dest() method to be able to use it in other modules Dec 22, 2025
@victoralmau
Copy link
Member Author

Shouldn't this be called _get_final_location_dest?

Okay, changed. The name of the method _get_location_final() was similar to the one that exists in sale_stock: https://github.com/odoo/odoo/blob/aad8a4a25f5d251b48e4f1894ee113176abdc834/addons/sale_stock/models/sale_order_line.py#L308

@pedrobaeza
Copy link
Member

Ah, OK, didn't know that analogy. You can comment that on the commit message/code for knowing about your inspiration. Yet, I think Odoo didn't choose the best name for it, don't you think? Anyway, if you think it's better to put the same name, just add the comment. What you prefer.

Copy link

@carlos-lopez-tecnativa carlos-lopez-tecnativa left a comment

Choose a reason for hiding this comment

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

LGTM

@victoralmau victoralmau force-pushed the 18.0-imp-rma-location_final branch from b2e7f0e to e1fa65e Compare December 22, 2025 11:07
@victoralmau victoralmau changed the title [18.0][IMP] rma: Add _get_final_location_dest() method to be able to use it in other modules [18.0][IMP] rma: Add _get_location_final() method to be able to use it in other modules Dec 22, 2025
@victoralmau
Copy link
Member Author

IMO the name _get_location_final() is not incorrect, because it represents reality, the value that will be defined in location_final_id. I have added the appropriate comment in the method to clarify this.

@pedrobaeza
Copy link
Member

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-515-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 48a6246 into OCA:18.0 Dec 22, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@pedrobaeza pedrobaeza deleted the 18.0-imp-rma-location_final branch December 22, 2025 11:25
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.

4 participants