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.

Sign up for GitHub

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

Jump to bottom

[14.0][IMP] sale_stock_mto_as_mts_orderpoint: return executed op #1694

Open
wants to merge 1 commit into
base: 14.0
Choose a base branch
from
Open

[14.0][IMP] sale_stock_mto_as_mts_orderpoint: return executed op #1694

wants to merge 1 commit into from

Conversation

TDu
Copy link
Member

@TDu TDu commented Sep 4, 2024

No description provided.

@rousseldenis
Copy link
Contributor

@TDu Prefer explicit naming sale_stock in commit message:

image

@rousseldenis rousseldenis added this to the 14.0 milestone Sep 4, 2024
jbaudoux
jbaudoux reviewed Sep 5, 2024
View reviewed changes
Copy link
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

I find it strange to return ids and not recordset

@TDu
Copy link
Member Author

TDu commented Sep 6, 2024

I find it strange to return ids and not recordset

True, I did not want to browse for the records when not needed.

@TDu TDu force-pushed the 14-imp-sale-stock-mto-as-mts-orderpoint-return-op branch from d698b4a to eae6545 Compare September 6, 2024 12:18
@TDu
Copy link
Member Author

TDu commented Sep 6, 2024

@TDu Prefer explicit naming sale_stock in commit message:

image

I updated the commit message. Always a juggle 🤹 between the length and the meaning on the title of the commit message.

@jbaudoux
Copy link
Contributor

jbaudoux commented Sep 6, 2024

I updated the commit message. Always a juggle 🤹 between the length and the meaning on the title of the commit message.

But the module name is with underscores and you put hyphens ...

@jbaudoux jbaudoux changed the title [14.0][IMP] s-s-mto-as-mts-orderpoint: return executed op [14.0][IMP] sale_stock_mto_as_mts_orderpoint: return executed op Sep 6, 2024
@TDu
[IMP] sale_stock_mto_as_mts_orderpoint
8ce633b 
Return the orderpoint ids that have been run on the sale confirmation.
@TDu TDu force-pushed the 14-imp-sale-stock-mto-as-mts-orderpoint-return-op branch from eae6545 to 8ce633b Compare September 6, 2024 13:54
@TDu
Copy link
Member Author

TDu commented Sep 6, 2024

I updated the commit message. Always a juggle 🤹 between the length and the meaning on the title of the commit message.

But the module name is with underscores and you put hyphens ...

🧟

@jbaudoux
Copy link
Contributor

I find it strange to return ids and not recordset

True, I did not want to browse for the records when not needed.

I would post the msg in the chatter in this module

@rousseldenis
Copy link
Contributor

@TDu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbaudoux jbaudoux jbaudoux left review comments

@rousseldenis rousseldenis rousseldenis approved these changes

Assignees
No one assigned
Labels
enhancement needs review
Projects
None yet
Milestone
14.0
Development

Successfully merging this pull request may close these issues.
3 participants
@TDu @rousseldenis @jbaudoux