Skip to content

Conversation

@Jamesbarford
Copy link
Contributor

  • More tests for the queue invariants. Adding them as guardrails and as cases we can re-implement to ensure the new mechanism behaves in the same way.
  • I also renamed a few things for clarity

@Jamesbarford Jamesbarford force-pushed the tests/tests-for-queue-ordering branch from 56cce4e to f62a596 Compare April 16, 2025 09:56
Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Looks great, having more tests is always super useful. Also adding comments to the code that explain what it does, because the logic is quite complicated. Let me know if you want to modify something else, otherwise LGTM.

@Jamesbarford
Copy link
Contributor Author

Looks great, having more tests is always super useful. Also adding comments to the code that explain what it does, because the logic is quite complicated. Let me know if you want to modify something else, otherwise LGTM.

I think for the time being I think this is enough. Presently I'm thinking that the logic from the function calculate_missing_from could be repurposed to insert unsorted data into a possible commit_queue table. I tired splitting the sort to a different step but it bloated out the PR and arguably made the logic harder to follow so I left the flow as it is and changed some variable names.

@Kobzol Kobzol merged commit a86adc1 into rust-lang:master Apr 16, 2025
11 checks passed
@Jamesbarford Jamesbarford deleted the tests/tests-for-queue-ordering branch April 16, 2025 13:19
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.

2 participants