Closed lemmy closed 1 year ago
The previous PR squashed all commits when merged into
main
. The commits in this PR are independent of each other. I'd suggest keeping the individual commits when merging.
Thanks for highlighting this. Squashing commits is the default for this repo. We try to aim for small incremental PRs (but sometimes fail, as demonstrated by yours truly with https://github.com/microsoft/CCF/pull/3965).
@achamayou Is this something we easily modify on a per PR basis? or what workaround would you suggest? Markus's commit messages include useful context on the changes made.
FYI: The motivation to extract/refactor what are essentially hardwired state constraints is strategically enabling simulation, liveness checking, bounded model-checking, and proofs. Also, a true state constraint causes a 2x slowdown in TLC because the state constraint is expensive to evaluate, and the number of generated goes up significantly (see https://github.com/lemmy/CCF/tree/mku-gh4264-3).
Thanks for highlighting this. Squashing commits is the default for this repo. We try to aim for small incremental PRs (but sometimes fail, as demonstrated by yours truly with #3965).
@achamayou Is this something we easily modify on a per PR basis? or what workaround would you suggest? Markus's commit messages include useful context on the changes made.
I discussed this with the team this morning and there is a preference for squashing for consistency with the other commits to main. The workflow for reviewing code history is to use git praise
to go to the PR and then review the PR description/comments/commits for a more fine-grained explanation.
This approach (along with a few other things) makes more sense for the production code than it does for the TLA+ spec. One solution is to put the TLA+ spec into a separate repo, but the idea behind having the spec live alongside the production code is to try to keep them both in sync (and maybe even enforce this one day with trace checking CI)
@lemmy adding to what @heidihoward has said: we can enable "Rebase and merge" as a choice when merging, and you/we/whoever merges this PR could make use of that. It's not our preference in general, but if you feel strongly about it, we'll do it :)
I have a few more commits pending that depend on the current commits in this PR. Do you want me to push those into this or open more PRs?
Opening seperate PRs is preferable if that's ok with you
/azp run
mku-gh4264-2@52623 aka 20221031.29 vs main ewma over 20 builds from 52062 to 52542
/azp run
This PR is ready to be merged (the failure in daily is unrelated)
The previous PR squashed all commits when merged into
main
. The commits in this PR are independent of each other. I'd suggest keeping the individual commits when merging.