Re: [PATCH] pull: abort by default if fast-forwarding is impossible

From: Alex Henrie <alexhenrie24@gmail.com>
Date: Jun 27, 2021, 19:54 UTC
Message-ID: <CAMMLpeQaY0z_baVQspJhAX0wanaJwneFoM5rG=xwFUmLEE-QaQ@mail.gmail.com>
In-Reply-To: <60d8ab7fe0761_ba5a2208b@natae.notmuch>

On Sat, Jun 26, 2021 at 7:34 PM Elijah Newren <newren@gmail.com> wrote:

>
> Thanks for working on this.

Thanks for all the feedback. I will incorporate your suggestions about the commit message and documentation into the next revision, whenever that is.

On Sat, Jun 26, 2021 at 10:12 PM Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 17 quoted lines

>
> Also, a bunch of tests are broken after this change:
>
>   t4013-diff-various.sh
>   t5521-pull-options.sh
>   t5524-pull-msg.sh
>   t5520-pull.sh
>   t5553-set-upstream.sh
>   t5604-clone-reference.sh
>   t6409-merge-subtree.sh
>   t6402-merge-rename.sh
>   t6417-merge-ours-theirs.sh
>   t7601-merge-pull-config.sh
>   t7603-merge-reduce-heads.sh
>
> If you didn't mean this patch to be applied then perhaps add the RFC
> prefix.

I actually did run `make test` before sending the patch, but when so many seemingly unrelated tests failed, I foolishly assumed that they were pre-existing failures. I should have run the tests on master for comparison, sorry. Or at least put "RFC" in the subject instead of "PATCH" as you suggest. I sincerely apologize for my lack of due diligence and I know that I need to do better at self-reviewing patches before sending them.

Show 12 quoted lines

> Alex Henrie wrote:
>
> > +           "You can replace \"git config\" with \"git config --global\" to set a default\n"
> > +           "preference for all repositories. You can also pass --rebase, --no-rebase,\n"
> > +           "or --ff-only on the command line to override the configured default per\n"
> > +           "invocation.\n"));
>
> Can I?
>
>   git -c pull.rebase=true pull --ff-only
>
> `--ff-only` doesn't seem to be overriding the configuration.

Ahh, so /that's/ what you meant by "3. Fix all the wrong behavior with --ff, --no-ff, and -ff-only". That does seem like something worth trying to fix before making the switch.

On Sat, Jun 26, 2021 at 10:16 PM Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 13 quoted lines

>
> Elijah Newren wrote:
>
> > The code changes look good, but you'll need to add several test
> > changes as well, or this code change will cause test failures.
>
> Wouldn't you consider sending a patch without running 'make test'
> "cavalier"?
>
> > Thanks for working on this.
>
> Such a completely different tone for a "cavalier" patch depending 100%
> on the person who sent it. Weird.

I'm trying to make things better, as I'm sure you are as well, and I know that I make a lot of mistakes and need the help of more experienced contributors like you. So please be nice, even if others are mean to you, because regardless of whether these comments were directed at me, this kind of comment just makes me feel like giving up.

On Sun, Jun 27, 2021 at 10:46 AM Felipe Contreras <felipe.contreras@gmail.com> wrote:

>
> I don't throw personal attacks, nor do I chastise contributors for
> attempting to improve the project. That's your department.

"That's your department" is a personal attack. How about we all go spend some time enjoying the weather, and then get back to working on Git problems later this week.

-Alex