beam-github mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <>
Subject [GitHub] [beam] apilloud commented on pull request #14729: [BEAM-9379] Update calcite to 1.26
Date Tue, 07 Sep 2021 01:29:17 GMT

apilloud commented on pull request #14729:

   I'm not sure what tools you are using to view history, but many of them have ways to give
your preferred view. For example `git log --first-parent`, `git blame --first-parent` etc.
   I squashed fixup commits and put effort into keeping the commits clean, if I hadn't it
would easily be 100 commits. I could have been more aggressive on squashing in a few cases
(I thought about squashing "Make it functional
   " into "Update to vendored Calcite to 1.26.0"), but I would also argue that at least "Update
to vendored Calcite to 1.26.0" is over squashed here. A single commit would not have been
appropriate in this case.
   For the two specific examples you've given: The "Update" doesn't represent any
single commit in the PR so I left it separate. The "Up spotbug stack size" is an issue exposed
by this change that is mostly unrelated, it could have been a separate PR (like #15362). Perhaps
I should have broken more of this out into separate PRs. This PR has been in the works for
over a year, and resulted in many split off changes: #13930 #14146 #14518 possibly others
I'm not remembering. (Also some of the commits in this change are being reverted in #15457.)
   I don't think we are going to agree on the proper curation of git history, I don't like
small "fixup" changes but I also think there are many cases where a PR can benefit from more
than a single commit. I proposed banning the "Squash and Merge" button in 2018:

This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail:

For queries about this service, please contact Infrastructure at:

View raw message