Skip to content

ScheduledMerges: some renaming and new trace messages #764

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

Merged
merged 3 commits into from
Jun 26, 2025

Conversation

jorisdral
Copy link
Collaborator

@jorisdral jorisdral commented Jun 19, 2025

Builds on top of #763 to prevent merge conflicts

The renamings were some changes I had lying around on an older branch, and the new trace messages I added to troubleshoot #755.

... and rename its constructors from `MergePolicyTiering` and
`MergePolicyLevelling` to `LevelTiering` and `LevelLevelling` respectively. This
makes the prototype follow the naming in the real implementation, and it makes
it possible to introduce `MergePolicy` as a table-wide configuration option
without name clashes.
This follows the naming from the real implementation, and it removes ambiguity
with `Op`s from `quickcheck-lockstep`.
@jorisdral jorisdral self-assigned this Jun 19, 2025
These messages helped with troubleshooting #755
@jorisdral jorisdral marked this pull request as ready for review June 19, 2025 10:11
Copy link
Collaborator

@mheinzel mheinzel left a comment

Choose a reason for hiding this comment

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

Looks good.

I'll rebase #709 on top of this.

where k = either lookUpVar id evk
ADelete var evk -> delete tr (lookUpVar var) k >> pure ()
ADelete var evk -> stToIO$ delete tr (lookUpVar var) k >> pure ()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ADelete var evk -> stToIO$ delete tr (lookUpVar var) k >> pure ()
ADelete var evk -> stToIO $ delete tr (lookUpVar var) k >> pure ()

Base automatically changed from jdral/issue755 to main June 26, 2025 13:57
@jorisdral jorisdral added this pull request to the merge queue Jun 26, 2025
Merged via the queue into main with commit 2f2fe36 Jun 26, 2025
30 checks passed
@jorisdral jorisdral deleted the jdral/proto-misc branch June 26, 2025 15:07
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