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

Improve remainder distribution logic #8062

Merged
merged 3 commits into from
Apr 20, 2024
Merged

Conversation

pupnewfster
Copy link
Member

Changes proposed in this pull request:

Improve remainder distribution to try and split the remainder as evenly as possible between the various destinations before falling back to sending to the first one it will fit in (#6617)

I also added some shortcuts to avoid attempting to send things and exit early if values hit zero.

…ly as possible between the

various destinations before falling back to sending to the first one it will fit in (#6617)
@thiakil
Copy link
Member

thiakil commented Apr 17, 2024

If we're being strict with tests, I'd expect to see one that verifies the amount sent to each endpoint (i.e. that the remainder was indeed split evenly)
Unless I've missed something, it's only testing that the full amount was sent 'somewhere'

@pupnewfster
Copy link
Member Author

I will add verification about how much it sends. The main reason I didn't is that one of the destinations can only accept a part of it, so figured if all of it got accepted that technically means it got distributed as expected. But you are right, it doesn't hurt to be more explicit.

@pupnewfster pupnewfster requested a review from thiakil April 20, 2024 00:20
@pupnewfster pupnewfster merged commit c53a41c into 1.20.4 Apr 20, 2024
3 checks passed
@pupnewfster pupnewfster deleted the remainder_distribution branch April 20, 2024 15:37
@pupnewfster pupnewfster added this to the 10.5.20 milestone Apr 20, 2024
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.

2 participants