Alex Henrie wrote:

On Sat, Jun 26, 2021 at 10:12 PM Felipe Contreras
[off-list ref] wrote:

quoted

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.

I personally don't see any need for apologies, we all make mistakes,
just keep it in mind for the future.

Personally I prefer to run prove instead, because the output is less
verbose, and there's a nice summary at the end:

  prove t[0-9][0-9][0-9][0-9]-*.sh

quoted

Alex Henrie wrote:

quoted

+           "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.

That is just one of the inconsistencies, there's many others.

Consider all these:

	git pull
	git pull --ff-only
	git -c pull.ff=only pull
	git -c pull.ff=only pull --ff-only

Do you see any good reason why the error message should be different for
any of these?

How about these?

	git pull
	git pull --ff

Why does --ff make the second command work?

Even worse when you start considering more combinations, like
"--no-ff --rebase" which is obviously nonsense, but it depends on the
configuration.

I proposed a solution for some of these inconsistencies with --ff*, but
was ignored [1].

On Sat, Jun 26, 2021 at 10:16 PM Felipe Contreras
[off-list ref] wrote:

quoted

Elijah Newren wrote:

quoted

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"?

quoted

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.

My mail was directed towards Elijah, not you.

I do appreciate all contributions, especially if they were done pro
bono.

On Sun, Jun 27, 2021 at 10:46 AM Felipe Contreras
[off-list ref] wrote:

quoted

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.

Stating facts about what a person did is not a personal attack, it's
just stating facts. Elijah did attacked me personally, that's a fact [2]:

  If I were in charge, at this point I would drop all of Felipe's
  patches on the floor (not just the ones from these threads), and not
  accept any further ones.  I am not in charge, though, and you have
  more patience than me.  But I will not be reviewing or responding to
  Felipe's patches further.  Please do not trust future submissions from
  him that have an "Acked-by" or "Reviewed-by" from me, including
  resubmissions of past patches.

And he did so after I made a "mistake" (in his view) that was much much
less serious than the one you did.

I am merely pointing out the inconsistency, that's all.

This has absolutely nothing to do with you, again... I appreciate you
used some of your free time to try to improve git pull.

Whatever beef Elijah has with me is an entirely different subject, that
I suggest you leave on the table. I don't think anything constructive
can come from trying to defend what he did.

Let's just focus on the patches.

Cheers.

[1] https://lore.kernel.org/git/5fdd154264baf_130e182082b@natae.notmuch/ (local)
[2] https://lore.kernel.org/git/CABPp-BG53Kd7MhzE3hdq5fjBQVV2Ew3skcUCAuTfM5daP2wmZA@mail.gmail.com/ (local)

-- 
Felipe Contreras