Alex Henrie wrote: > On Sat, Jun 26, 2021 at 10:12 PM Felipe Contreras > <felipe.contreras@xxxxxxxxx> wrote: > > > > 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 > > 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. 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 > <felipe.contreras@xxxxxxxxx> wrote: > > > > 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. 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 > <felipe.contreras@xxxxxxxxx> 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. 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/ [2] https://lore.kernel.org/git/CABPp-BG53Kd7MhzE3hdq5fjBQVV2Ew3skcUCAuTfM5daP2wmZA@xxxxxxxxxxxxxx/ -- Felipe Contreras
