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

Sales commission so based module #471

Closed

Conversation

maksymkv25
Copy link

No description provided.

@maksymkv25
Copy link
Author

@pedrobaeza Hi, the module is ready. Do I need to do something extra?

@pedrobaeza pedrobaeza added this to the 16.0 milestone Oct 25, 2023
@pedrobaeza
Copy link
Member

Formally, you can squash commits together, as they don't add value being unfolded. On the contributing process, at least 2 persons must review the PR, one of them being PSC:

https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#review

Unfortunately, I can't physically review every PR that enters OCA. You can enroll people to review your PR reviewing other existing PRs and asking in exchange that they review yours.

� This is the 1st commit message:

[ADD] sale_commission_so_based module

� The commit message OCA#2 will be skipped:

� [IMP] readme

� The commit message OCA#3 will be skipped:

� [REF] autoflake fixes, dependency added

� The commit message OCA#4 will be skipped:

� [FIX] Inline emphasis start-string without end-string.

� The commit message OCA#5 will be skipped:

� [FIX] Inline emphasis start-string without end-string.

� The commit message OCA#6 will be skipped:

� [REM] report_settlement_templates.xml

� The commit message OCA#7 will be skipped:

� [REF] import

� The commit message OCA#8 will be skipped:

� [REF] import

� The commit message OCA#9 will be skipped:

� [IMP] increase test coverage

� The commit message OCA#10 will be skipped:

� [IMP] pylint fixes

� The commit message OCA#11 will be skipped:

� [IMP] adopting for checks

� The commit message OCA#12 will be skipped:

� [IMP] adopting for checks
@maksymkv25 maksymkv25 marked this pull request as draft October 25, 2023 08:39
@maksymkv25 maksymkv25 marked this pull request as ready for review October 25, 2023 08:39
@maksymkv25
Copy link
Author

@dcorio @joao-p-marques
Hello, can you please review this PR? Thank you

@francesco-ooops
Copy link
Contributor

@maksymkv25 you should make sure PR is compliant with https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst before asking for reviews

Also there is no documentation for this module, so functionals can't provide their opinion on whether it works or not

@jguenat
Copy link
Member

jguenat commented Nov 2, 2023

Hello @maksymkv25 ,
Thanks for your PR, I tested the module quickly and it looks promising.

Concerning documentation you should create a readme folder containing .rst files (USAGE.rst etc...). Then the oca bot will create the main README.rst automatically after merge (you can do it manually if you want https://github.com/OCA/maintainer-tools#readme-generator).

Copy link
Member

@jguenat jguenat left a comment

Choose a reason for hiding this comment

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

Also it would be grate to have the "Source sale line" on the commission.settlement view & report, same as what is done with the Source invoice line in account_commission.

sale_commission_so_based/models/commission_settlement.py Outdated Show resolved Hide resolved
Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

Functional ok

squash commits please

@maksymkv25
Copy link
Author

This PR will be closed, because had a problem whit heads to squash commits.
New PR

@maksymkv25 maksymkv25 closed this Nov 9, 2023
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.

4 participants