SGTM.
On Fri, Jul 14, 2017 at 3:37 PM, Kenneth Knowles <k...@google.com.invalid> wrote: > I think 1 & 3 can be squashed :-) > > If someone has fixup! commits you definitely don't want to merge exactly > as-is so it shouldn't be possible. (and if they messed up their fixup! so > it doesn't vanish, you also don't want to merge that) > > And then, yes, that covers my wants. > > On Fri, Jul 14, 2017 at 3:00 PM, Jason Kuster < > jasonkus...@google.com.invalid> wrote: > >> Hey folks, thanks for your thoughts! >> >> I have some responses for you. :) >> >> Regarding squashing, it looks like there are three things at play -- >> forgive me if I've misunderstood. >> >> 1. Merge commits exactly as they are in the PR. >> 2. Squash all commits down to the first commit >> 3. Automatically squash fixup! and squash! commits but leave things as they >> are otherwise. >> >> Is the prevailing sentiment to enable all three of these, or just two? >> >> To address Ismaël's comment about gaining insight into what MergeBot is >> doing, I have two suggestions. a) MergeBot can comment back the list of >> commands that it took in the case that it failed. b) MergeBot can comment a >> link to the merge log for the PR in question. We already capture this and >> put it somewhere internet-accessible, it's just not particularly >> discoverable. This way people could watch the actual STDOUT as MergeBot is >> operating, or after it fails to merge a PR. Would that help? >> >> Also for the curious, I'm tracking all the future MergeBot work I can think >> of in the doc linked below (I may wind up filing tickets for some of this >> stuff at some point, but am more likely to track via github issues on the >> mergebot repository for now). Comments welcome. :) >> >> https://docs.google.com/document/d/13D1nUgTeonyvNtRi4bJM- >> Vyj9YOCVHZT7QA6EOauKT4/edit >> >> On Wed, Jul 12, 2017 at 1:04 PM, Kenneth Knowles <k...@google.com.invalid> >> wrote: >> >> > On Wed, Jul 12, 2017 at 12:08 PM, Robert Bradshaw < >> > rober...@google.com.invalid> wrote: >> > >> > > On Tue, Jul 11, 2017 at 7:14 PM, Kenneth Knowles >> <k...@google.com.invalid >> > > >> > > wrote: >> > > >> > > > The thing is that "fixup! <prior subject line>" indicates that this >> > fixup >> > > > should be reordered and applied to the referenced commit. Squashing >> in >> > > > order is not correct. I think the bot reordering to squash is not a >> > good >> > > > idea. >> > > >> > > I don't see why reordering in this case is a bad thing (if it applies >> > > cleanly--one could even automatically check that the patches commute). >> > > >> > > > So maybe I wasn't clear about the options I want. I want both of: >> > > > >> > > > (1) The bot merges the commits exactly as they are (for the >> > > > git-knowledgable) >> > > > (2) The bot squashes all the commits in order (for casual >> contributors) >> > > > >> > > > Way simpler than anything interactive and with no reording by the >> bot. >> > > > The rest of my thoughts were just ways to further avoid messing this >> > up. >> > > >> > > Yeah, these are the most common, and I highly agree should make it >> > > hard to accidentally merge fixup commits. I would like to support the >> > > (common) case of an advanced user having fixup commits in the review, >> > > but being able to merge without waiting for her to manually squash >> > > them after the LGTM. >> > > >> > >> > OK, yea, I think it is fair to allow the bot to try to reorder if it goes >> > cleanly. So sounds good to me. >> > >> > Kenn >> > >> >> >> >> -- >> ------- >> Jason Kuster >> Apache Beam / Google Cloud Dataflow >>
