| Message ID | 20210621175234.1079004-3-felipe.contreras@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | pull: documentation improvements | expand |
On Mon, Jun 21, 2021 at 11:52 AM Felipe Contreras <felipe.contreras@gmail.com> wrote: > > + "If unsure, run \"git pull --no-rebase\".\n" I don't think the message should recommend merging over rebasing; which strategy is the correct one depends entirely on the project's workflow and the user's role within that workflow. The eventual goal is to get rid of the default here and make the user make an educated choice, which does imply some work on the user's part, but it avoids the massive headaches created by users merging without understanding what they're doing. -Alex
Alex Henrie wrote: > On Mon, Jun 21, 2021 at 11:52 AM Felipe Contreras > <felipe.contreras@gmail.com> wrote: > > > > + "If unsure, run \"git pull --no-rebase\".\n" > > I don't think the message should recommend merging over rebasing; This is the default strategy. Doing `git pull` is the same as doing `git pull --no-rebase`, except with the warning. > The eventual goal is to get rid of the default here and make the user > make an educated choice, which does imply some work on the user's > part, but it avoids the massive headaches created by users merging > without understanding what they're doing. Indeed, but any minute change in git's UI is a gargantuan task that takes several years--or even decades--to accomplish, if it ever happens. I started this patch in 2013, and here we are. This patch series is not attempting to do the impossible. Cheers.
On Mon, Jun 21, 2021 at 12:51 PM Felipe Contreras <felipe.contreras@gmail.com> wrote: > > Alex Henrie wrote: > > On Mon, Jun 21, 2021 at 11:52 AM Felipe Contreras > > <felipe.contreras@gmail.com> wrote: > > > > > > + "If unsure, run \"git pull --no-rebase\".\n" > > > > I don't think the message should recommend merging over rebasing; > > This is the default strategy. Yes, but it shouldn't be, and we shouldn't make the problem worse by encouraging people to default to merging without thinking. > > The eventual goal is to get rid of the default here and make the user > > make an educated choice, which does imply some work on the user's > > part, but it avoids the massive headaches created by users merging > > without understanding what they're doing. > > Indeed, but any minute change in git's UI is a gargantuan task that > takes several years--or even decades--to accomplish, if it ever happens. > I started this patch in 2013, and here we are. Although what needs to be done had been envisioned by some as early as 2013, the warning has only been around since Git 2.27 (released in June 2020), and it was only restricted to pulls where fast-forwarding is impossible in Git 2.31 (released in March 2021). The good news is that (unless I'm mistaken) there are no more changes that need to be made prior to changing the message from from "advise" to "die". All that needs to be done is to set a date to make the switch. For comparison, users were given from Git 1.8 to Git 2.0 (October 2012 to May 2014, 1 year and 7 months) to acclimate when push.default changed from "matching" to "simple". So how about we plan to stop merging by default in Git 2.40 (due around the end of 2022 or beginning of 2023), and update the warning message to advise the users of the pending behavioral change? -Alex
Alex Henrie wrote: > On Mon, Jun 21, 2021 at 12:51 PM Felipe Contreras > <felipe.contreras@gmail.com> wrote: > > > > Alex Henrie wrote: > > > On Mon, Jun 21, 2021 at 11:52 AM Felipe Contreras > > > <felipe.contreras@gmail.com> wrote: > > > > > > > > + "If unsure, run \"git pull --no-rebase\".\n" > > > > > > I don't think the message should recommend merging over rebasing; > > > > This is the default strategy. > > Yes, but it shouldn't be, Feel free to propose a patch to change that, in the meantime this is the default. > and we shouldn't make the problem worse by encouraging people to > default to merging without thinking. We are not. > > > The eventual goal is to get rid of the default here and make the user > > > make an educated choice, which does imply some work on the user's > > > part, but it avoids the massive headaches created by users merging > > > without understanding what they're doing. > > > > Indeed, but any minute change in git's UI is a gargantuan task that > > takes several years--or even decades--to accomplish, if it ever happens. > > I started this patch in 2013, and here we are. > > Although what needs to be done had been envisioned by some as early as > 2013, the warning has only been around since Git 2.27 (released in > June 2020), and it was only restricted to pulls where fast-forwarding > is impossible in Git 2.31 (released in March 2021). The good news is > that (unless I'm mistaken) there are no more changes that need to be > made prior to changing the message from from "advise" to "die". There is *a lot* that needs to be done. 1. Update the documentation 2. Add a --merge option (instead of the ackward --no-rebase) 3. Fix all the wrong behavior with --ff, --no-ff, and -ff-only 4. Add a pull.mode configuration 5. Add a configuration for the mode in which we want to die 6. Fix inconsistencies in the UI Only *then* can we even begin to throw a warning stating that the default behavior might change in the future, and how to try the new behavior. > All that needs to be done is to set a date to make the switch. No, there's a lot more. > For comparison, users were given from Git 1.8 to Git 2.0 (October 2012 > to May 2014, 1 year and 7 months) to acclimate when push.default > changed from "matching" to "simple". We haven't told the users the default mode is going to change for a single day. We don't have a configuration to tell them to turn on to enable the new mode, nor do we have a configuration to tell them to enable to stay in the old mode. There's no configuration in git 2.32 that is equivalent to what I proposed with pull.mode=fast-forward? In the meantime there's no reason to have subpar documentation.
On Mon, Jun 21, 2021 at 4:12 PM Felipe Contreras <felipe.contreras@gmail.com> wrote: > > Alex Henrie wrote: > > On Mon, Jun 21, 2021 at 12:51 PM Felipe Contreras > > <felipe.contreras@gmail.com> wrote: > > > > > > Alex Henrie wrote: > > > > The eventual goal is to get rid of the default here and make the user > > > > make an educated choice, which does imply some work on the user's > > > > part, but it avoids the massive headaches created by users merging > > > > without understanding what they're doing. > > > > > > Indeed, but any minute change in git's UI is a gargantuan task that > > > takes several years--or even decades--to accomplish, if it ever happens. > > > I started this patch in 2013, and here we are. > > > > Although what needs to be done had been envisioned by some as early as > > 2013, the warning has only been around since Git 2.27 (released in > > June 2020), and it was only restricted to pulls where fast-forwarding > > is impossible in Git 2.31 (released in March 2021). The good news is > > that (unless I'm mistaken) there are no more changes that need to be > > made prior to changing the message from from "advise" to "die". > > There is *a lot* that needs to be done. > > 1. Update the documentation > 2. Add a --merge option (instead of the ackward --no-rebase) > 3. Fix all the wrong behavior with --ff, --no-ff, and -ff-only > 4. Add a pull.mode configuration > 5. Add a configuration for the mode in which we want to die > 6. Fix inconsistencies in the UI I agree with you that the documentation should be updated when the change is made (#1), and maybe there should be a config option to go back to the behavior of warning but doing the merge anyway (#5). The rest I think are things that would be nice to have but don't preclude making the switch because aborting instead of merging would not introduce any new UI limitations or inconsistencies. Of course, it's ultimately up to Junio and the wider Git community, and I would love to hear their thoughts about it. > In the meantime there's no reason to have subpar documentation. My only serious objection to this patch is the instruction to merge if you don't know what to do instead of asking the repository maintainer what to do or reading the Git documentation. I don't have a strong opinion on the rest of the patch. -Alex
Alex Henrie wrote: > On Mon, Jun 21, 2021 at 4:12 PM Felipe Contreras > <felipe.contreras@gmail.com> wrote: > > > > Alex Henrie wrote: > > > Although what needs to be done had been envisioned by some as early as > > > 2013, the warning has only been around since Git 2.27 (released in > > > June 2020), and it was only restricted to pulls where fast-forwarding > > > is impossible in Git 2.31 (released in March 2021). The good news is > > > that (unless I'm mistaken) there are no more changes that need to be > > > made prior to changing the message from from "advise" to "die". > > > > There is *a lot* that needs to be done. > > > > 1. Update the documentation > > 2. Add a --merge option (instead of the ackward --no-rebase) > > 3. Fix all the wrong behavior with --ff, --no-ff, and -ff-only > > 4. Add a pull.mode configuration > > 5. Add a configuration for the mode in which we want to die > > 6. Fix inconsistencies in the UI > > I agree with you that the documentation should be updated when the > change is made (#1), I'm saying the documentation needs to be updated _before_ the change is made. There's no reason not to have the fast-forward example in the documentation. > and maybe there should be a config option to go back to the behavior > of warning but doing the merge anyway (#5). Before that we need a configuration to turn the behavior on. > The rest I think are things that would be nice to have but don't > preclude making the switch because aborting instead of merging would > not introduce any new UI limitations or inconsistencies. They don't preclude the switch, but the switch should be precluded by a warning, and the warning would be something like: The pull was not fast-forward, in the future you will have to choose a merge, or a rebase. To quell this message you have two main options: 1. Adopt the new behavior: git config --global pull.mode ff-only 2. Maintain the current behavior: git config --global pull.mode merge For now we will fall back to the traditional behavior (merge). Read "git pull --help" for more information. Without having the changes I listed this warning can't be as useful. > Of course, it's ultimately up to Junio and the wider Git community, > and I would love to hear their thoughts about it. I would not hold my breath. > > In the meantime there's no reason to have subpar documentation. > > My only serious objection to this patch is the instruction to merge if > you don't know what to do instead of asking the repository maintainer > what to do or reading the Git documentation. I don't have a strong > opinion on the rest of the patch. I would be fine if the patch is merged without that line, but I believe the patch is better with that line. The line doesn't say "do this this if you don't know what to do", it says if you are *unsure*. That is not the same thing. And the user *already* did a merge, as that's what 'git pull' does by default. The advice throws a warning, but proceeds with the merge. The only thing the line is telling the user is how to muffle the message if she is unsure of the previous muffling options. Would you be happier with this? The simplest way to maintain the current behavior is to just do "git pull --no-rebase". Cheers.
On Mon, Jun 21, 2021 at 8:15 PM Alex Henrie <alexhenrie24@gmail.com> wrote: > > My only serious objection to this patch is the instruction to merge if > you don't know what to do instead of asking the repository maintainer > what to do or reading the Git documentation. I don't have a strong > opinion on the rest of the patch. You're not alone, Alex; I objected to that part as well. (See e.g. https://lore.kernel.org/git/CABPp-BF4rXBOKsn8bG6y3QUEtNVV9K2Pk5NmwrU5818CqhRt_Q@mail.gmail.com/ and various other emails in that thread, ending with "agree to disagree" later). I still object to it as I did then. I'm curious whether it'll just be resubmitted again multiple times, eventually with a cover letter that repeats something along the lines of "these are the non-controversial changes from last-year series which...don't have any reason not to be merged."
On Tue, Jun 22, 2021 at 9:06 AM Elijah Newren <newren@gmail.com> wrote: > > On Mon, Jun 21, 2021 at 8:15 PM Alex Henrie <alexhenrie24@gmail.com> wrote: > > > > My only serious objection to this patch is the instruction to merge if > > you don't know what to do instead of asking the repository maintainer > > what to do or reading the Git documentation. I don't have a strong > > opinion on the rest of the patch. > > You're not alone, Alex; I objected to that part as well. (See e.g. > https://lore.kernel.org/git/CABPp-BF4rXBOKsn8bG6y3QUEtNVV9K2Pk5NmwrU5818CqhRt_Q@mail.gmail.com/ > and various other emails in that thread, ending with "agree to > disagree" later). I still object to it as I did then. Thanks for the link, and sorry for not having followed this conversation closely enough to have seen your previous replies. While we're on the subject, do you have any thoughts on what (if anything) more should be done before making the switch to aborting instead of merging with a warning in `git pull`? -Alex
Elijah Newren wrote: > On Mon, Jun 21, 2021 at 8:15 PM Alex Henrie <alexhenrie24@gmail.com> wrote: > > > > My only serious objection to this patch is the instruction to merge if > > you don't know what to do instead of asking the repository maintainer > > what to do or reading the Git documentation. I don't have a strong > > opinion on the rest of the patch. > > You're not alone, Alex; I objected to that part as well. (See e.g. > https://lore.kernel.org/git/CABPp-BF4rXBOKsn8bG6y3QUEtNVV9K2Pk5NmwrU5818CqhRt_Q@mail.gmail.com/ > and various other emails in that thread, ending with "agree to > disagree" later). I still object to it as I did then. You made your disagreement known in [1], I responded to it with a devastating argument in [2], and you immediately withtdrew from the discussion in [3] without engaging my argument at all. In total you engaged with my arguments zero times. This is all you replied: Yes, we can agree to disagree on this particular point. To make the record straight, I'm restating the argument you avoided in full: But that is not the warning, this is the warning: Pulling without specifying how to reconcile divergent branches is discouraged; you need to specify if you want a merge, a rebase, or a fast-forward. You can squelch this message by running one of the following commands: git config pull.rebase false # merge (the default strategy) git config pull.rebase true # rebase git config pull.ff only # fast-forward only You can replace "git config" with "git config --global" to set a default preference for all repositories. If unsure, run "git pull --merge". Read "git pull --help" for more information. This warning says: 1. There's 3 options: merge, rebase, fast-forward 2. merge is the default strategy 3. If unsure, specify --merge (the default strategy) So taken altogether it does say what is the default strategy. > More > importantly, it makes a recommendation...and one that undercuts the > point of the message. So? When boarding a plane the flight attendants do a safety demonstration that passengers should pay attention to. If one passenger is not paying attention (listening to music on headphones, and reading a book) what should the crew do? 1. Remove the passenger's headphones and force him to pay attention to the safety demonstration 2. Let the passenger ignore the safety demonstration Human beings are independent agents responsible for their own actions. You as a separate human being--a crew member--can argue that it's not in the best interest of the passenger to ignore the safety demonstration, and you may be right, but the passenger decisions are still the passenger's decisions, even if they are bad. Do you think the crew should disregard the passenger's volition and force him to pay attention to the safety demonstration? > It makes it feel like the message shouldn't > exist at all in any circumstances. I even suspect that adding that > sentence may undercut any efforts towards changing the default to > ff-only-as-default. While I'm a big fan of most of what you've done > in this series, I will object to its merging for as long as this > stays. (I definitely don't have veto power or anything close to it, > just stating what my opinion is.) The current warning should not exist at all. The complaint from Vít Ondruch [1] that reignited this series is a valid one. A *permanent* warning is not good. We should have a *temporary* warning with the express purpose of notifying users of an upcoming change. If we have not yet decided on what should be the default (Junio seems to have casted some doubt on the consensus [2]), and we don't have a clear path forward to implement such change (we can't even tell users to use "pull.ff=only", since eventually it may be "pull.mode=ff-only"), then we must remove the warning. It was a mistake to put a *permanent* warning before deciding to change the default. So, there's two options: 1. We decide on a path forward and fix the warning so it *temporarily* explains what will happen in the future 2. We remove the *permanent* warning Since we are already here, we might as well take advantage of that warning and repurpose it. But in the meantime--while the git project decides what to do, and what configurations to suggest the users to change--we should at the very least waste as little as the user's time as possible, and give him/her a quick opt-out. Yes, a quick opt-out defeats the purpose of a warning, but we must respect the users' volition. The user may be on a deadline trying to push some changes to production before the weekend, and after a system update be annoyed with this warning on every pull. The user may not have time to look at the warning, decide he wants to read the warning in the future, maybe next Monday, and thus not configure anything to silence it. What's wrong with a user saying "I don't have time for this now, please tell me what to do for now, I'll look at the warning later"? If anything for those users the configuration is the wrong thing to do, because being in a hurry they just choose the first configuration and forget about the warning without actually looking at it (because they didn't have time), and it will not appear any more. By typing "git pull --merge" the user can get rid of the warning *for now*, but the next time he does "git pull" the warning will reappear, and at that time perhaps the user does have the time to read it, and look at the manpage. Nobody likes their workflow to be interrupted and be forced to do anything. I don't think my patches plus that suggestion for a quick opt-out are in any way worse than the current situation. If you think they are, then we'll just have to agree to disagree. I quote the voice of Vít Ondruch, which I think represents the typical user: "please select any strategy considered more appropriate and stop warning me". Cheers. [1] https://lore.kernel.org/git/742df4c2-2bc5-8a4b-8de1-cd5e48718398@redhat.com/ [2] https://lore.kernel.org/git/xmqqh7p1fjml.fsf@gitster.c.googlers.com/ > I'm curious whether it'll just be resubmitted again multiple times, > eventually with a cover letter that repeats something along the lines > of "these are the non-controversial changes from last-year series > which...don't have any reason not to be merged." The fact that **one** person was not 100% on board with a change doesn't make it controversial. You made the conscious choice to withdraw from the discussion immediately, so just like a person who abandons an election cycle and decides not to vote, you are leving the future of the matter in the hands of others. If you want to rejoin the discussion, feel free to respond to my argument that you dodged last round [3]. Cheers. [1] https://lore.kernel.org/git/CABPp-BF4rXBOKsn8bG6y3QUEtNVV9K2Pk5NmwBF4rXBOKsn8bG6y3QUEtNVV9K2Pk5NmwrU5818CqhRt_QrU5818CqhRt_Q@mail.gmail.com/ [2] https://lore.kernel.org/git/CAMP44s2L24jhCG9ps72--ZiJkXUovR726jCf8JTLHAs0jV7Whg@mail.gmail.com/ [3] https://lore.kernel.org/git/CABPp-BGdNt8TBMTE9zvaicF5AtvyTBhpiJXqkuZc7mBLGbw0Qw@mail.gmail.com/
On Tue, Jun 22, 2021 at 2:22 PM Alex Henrie <alexhenrie24@gmail.com> wrote: > > On Tue, Jun 22, 2021 at 9:06 AM Elijah Newren <newren@gmail.com> wrote: > > > > On Mon, Jun 21, 2021 at 8:15 PM Alex Henrie <alexhenrie24@gmail.com> wrote: > > > > > > My only serious objection to this patch is the instruction to merge if > > > you don't know what to do instead of asking the repository maintainer > > > what to do or reading the Git documentation. I don't have a strong > > > opinion on the rest of the patch. > > > > You're not alone, Alex; I objected to that part as well. (See e.g. > > https://lore.kernel.org/git/CABPp-BF4rXBOKsn8bG6y3QUEtNVV9K2Pk5NmwrU5818CqhRt_Q@mail.gmail.com/ > > and various other emails in that thread, ending with "agree to > > disagree" later). I still object to it as I did then. > > Thanks for the link, and sorry for not having followed this > conversation closely enough to have seen your previous replies. While No worries, you were trying to be a good citizen by reviewing patches, and the patches didn't come with links to the old threads (even if you recursively followed links provided in each email as far as I can tell), so I wouldn't expect you to know. But I saw you expressing similar sentiments as I had previously so I dug out my old email and linked it. > we're on the subject, do you have any thoughts on what (if anything) > more should be done before making the switch to aborting instead of > merging with a warning in `git pull`? I think Junio already answered that over here: https://lore.kernel.org/git/xmqq360h8286.fsf@gitster.c.googlers.com/ (he discussed it multiple times in that thread, but hopefully that's a good enough example).
Elijah Newren wrote: > On Tue, Jun 22, 2021 at 2:22 PM Alex Henrie <alexhenrie24@gmail.com> wrote: > > > > On Tue, Jun 22, 2021 at 9:06 AM Elijah Newren <newren@gmail.com> wrote: > > > > > > On Mon, Jun 21, 2021 at 8:15 PM Alex Henrie <alexhenrie24@gmail.com> wrote: > > > > > > > > My only serious objection to this patch is the instruction to merge if > > > > you don't know what to do instead of asking the repository maintainer > > > > what to do or reading the Git documentation. I don't have a strong > > > > opinion on the rest of the patch. > > > > > > You're not alone, Alex; I objected to that part as well. (See e.g. > > > https://lore.kernel.org/git/CABPp-BF4rXBOKsn8bG6y3QUEtNVV9K2Pk5NmwrU5818CqhRt_Q@mail.gmail.com/ > > > and various other emails in that thread, ending with "agree to > > > disagree" later). I still object to it as I did then. > > > > Thanks for the link, and sorry for not having followed this > > conversation closely enough to have seen your previous replies. While > > No worries, you were trying to be a good citizen by reviewing patches, > and the patches didn't come with links to the old threads That's not true. This patch series [1] came with a link to the previous patch series [2]. I didn't, cannot, and shouldn't contain hundreds of links to the hundres of responses to this topic over the past 10 years. [1] https://lore.kernel.org/git/20210621175234.1079004-1-felipe.contreras@gmail.com/ [2] https://lore.kernel.org/git/20201218211026.1937168-1-felipe.contreras@gmail.com/
On Tue, Jun 22, 2021 at 9:18 PM Felipe Contreras <felipe.contreras@gmail.com> wrote: > > Elijah Newren wrote: > > On Tue, Jun 22, 2021 at 2:22 PM Alex Henrie <alexhenrie24@gmail.com> wrote: > > > .... > > > Thanks for the link, and sorry for not having followed this > > > conversation closely enough to have seen your previous replies. While > > > > No worries, you were trying to be a good citizen by reviewing patches, > > and the patches didn't come with links to the old threads > > That's not true. This patch series [1] came with a link to the previous > patch series [2]. > > I didn't, cannot, and shouldn't contain hundreds of links to the hundres > of responses to this topic over the past 10 years. Sorry for being unclear; by links I meant either direct or indirect ones. Your series came with a link to the previous series. The previous series, however, only contained a link to a series you were basing upon rather than to a series which contained the changes you were resubmitting. Thus, following the links in each submission would not get you back to the old discussions (I double checked). Also, this particular point was not meant as a critique of your current submission. I don't even think it's all that big a deal for the previous submission either (it's an easy oversight to make given that Junio submitted a portion of one of your old series). My point was simply that Alex didn't need to feel bad for not having been aware of all the old discussions; even if he had tried to dive deep by recursively following all the links, he wouldn't find it.
On Tue, Jun 22, 2021 at 6:09 PM Felipe Contreras <felipe.contreras@gmail.com> wrote: > > Elijah Newren wrote: ... > > You're not alone, Alex; I objected to that part as well. (See e.g. > > https://lore.kernel.org/git/CABPp-BF4rXBOKsn8bG6y3QUEtNVV9K2Pk5NmwrU5818CqhRt_Q@mail.gmail.com/ > > and various other emails in that thread, ending with "agree to > > disagree" later). I still object to it as I did then. > > You made your disagreement known in [1], I responded to it with a > devastating argument in [2], and you immediately withtdrew from the > discussion in [3] without engaging my argument at all. I didn't find anything new or persuasive in your rehashing of your arguments. I had stated my disagreement twice already, and having us both repeat our arguments does no one any good, so I just stated we can agree to disagree. > > I'm curious whether it'll just be resubmitted again multiple times, > > eventually with a cover letter that repeats something along the lines > > of "these are the non-controversial changes from last-year series > > which...don't have any reason not to be merged." > > The fact that **one** person was not 100% on board with a change doesn't > make it controversial. This is a disconcerting response. I would have thought perhaps you might say "Whoops, forgot about that part of the thread", or "Sorry, didn't mean to include that line". Perhaps I shouldn't be surprised that you instead decided to try to redefine goal posts, but it's still discouraging. I also find your characterization of the old thread disappointing; I clearly cared enough to state my objection in three separate emails, so it's more than just "not 100% on board". And Junio referred to the analogy in your "devastating argument" as "irrelevant", so it's not clear you convinced others either. > You made the conscious choice to withdraw from the discussion > immediately, so just like a person who abandons an election cycle and > decides not to vote, you are leving the future of the matter in the > hands of others. This is quite a disappointing argument. If this position were to be accepted broadly within the project, it would suggest scorched-earth last-man standing tactics -- just arguing until the other side runs out of energy. If that was used to determine our forward strategy, it'd result in a massive waste of energy, people feeling drained and losing motivation to contribute, some people just deciding to leave the project, and a myriad of other negative outcomes. In fact, occurrences of such behavior has already had such outcomes. Rehashing the same arguments repeatedly damages the discourse within the project as well as the project itself. There's no point in doing so.
Elijah Newren wrote: > On Tue, Jun 22, 2021 at 9:18 PM Felipe Contreras > <felipe.contreras@gmail.com> wrote: > > > > Elijah Newren wrote: > > > On Tue, Jun 22, 2021 at 2:22 PM Alex Henrie <alexhenrie24@gmail.com> wrote: > > > > > .... > > > > Thanks for the link, and sorry for not having followed this > > > > conversation closely enough to have seen your previous replies. While > > > > > > No worries, you were trying to be a good citizen by reviewing patches, > > > and the patches didn't come with links to the old threads > > > > That's not true. This patch series [1] came with a link to the previous > > patch series [2]. > > > > I didn't, cannot, and shouldn't contain hundreds of links to the hundres > > of responses to this topic over the past 10 years. > > Sorry for being unclear; by links I meant either direct or indirect > ones. Your series came with a link to the previous series. The > previous series, however, only contained a link to a series you were > basing upon rather than to a series which contained the changes you > were resubmitting. Thus, following the links in each submission would > not get you back to the old discussions (I double checked). That's true, but there's no combination of links that would get you all the discussions. There were 4 different approaches attempted, each one with their own thread. Also, since other people didn't follow the threads correctly I had to send different versions of those threads. > Also, this particular point was not meant as a critique of your > current submission. I don't even think it's all that big a deal for > the previous submission either (it's an easy oversight to make given > that Junio submitted a portion of one of your old series). My point > was simply that Alex didn't need to feel bad for not having been aware > of all the old discussions; even if he had tried to dive deep by > recursively following all the links, he wouldn't find it. He was aware of the old discussions, as he was part of them, but he chose to disengage from them [1]. Either way I don't see why would anyone should feel bad about not being aware of all the hundreds of responses over dozens of threads over several years. All we can ask from contributors giving their feedback for free is their attempt to do their best, nothing more. Cheers. [1] http://lore.kernel.org/git/CAMMLpeQA7VW_C4yw_8n6j_SCoGr8k4VUOUaEp98UxUAMR6-MVw@mail.gmail.com
Elijah Newren wrote: > On Tue, Jun 22, 2021 at 6:09 PM Felipe Contreras > <felipe.contreras@gmail.com> wrote: > > > > Elijah Newren wrote: > ... > > > You're not alone, Alex; I objected to that part as well. (See e.g. > > > https://lore.kernel.org/git/CABPp-BF4rXBOKsn8bG6y3QUEtNVV9K2Pk5NmwrU5818CqhRt_Q@mail.gmail.com/ > > > and various other emails in that thread, ending with "agree to > > > disagree" later). I still object to it as I did then. > > > > You made your disagreement known in [1], I responded to it with a > > devastating argument in [2], and you immediately withtdrew from the > > discussion in [3] without engaging my argument at all. > > I didn't find anything new or persuasive in your rehashing of your > arguments. The fact that you don't find something persuasive doesn't mean there wasn't anything persuasive. Moreover, the argument was completely new. > I had stated my disagreement twice already, and having us both repeat > our arguments does no one any good, so I just stated we can agree to > disagree. I did not repeat my argument, I made a completely new argument. > > > I'm curious whether it'll just be resubmitted again multiple times, > > > eventually with a cover letter that repeats something along the lines > > > of "these are the non-controversial changes from last-year series > > > which...don't have any reason not to be merged." > > > > The fact that **one** person was not 100% on board with a change doesn't > > make it controversial. > > This is a disconcerting response. Why? That's the definition of the word "controversial". In order for X to be controversial a substantive amount of people need to have opposing views. The fact that a few people deny the roundness of the Earth doesn't make that idea "controversial". Similarly, the fact that one person disagrees with X doesn't make X controversial. > I also find your characterization of the old thread disappointing; I > clearly cared enough to state my objection in three separate emails, > so it's more than just "not 100% on board". I said "change", not "thread". And, you were on board with 23 out of 24 lines of the patch. So you were 96% on board. That's just a mathematical fact. > And Junio referred to the analogy in your "devastating argument" as > "irrelevant", so it's not clear you convinced others either. Junio is not infallible. The fact that Junio said X was irrelevant doesn't mean X was irrelevant. And in this particular case he was wrong, as I explained in [1], because he equated apples (regulations) to oranges (policy). To be specific: this is a false equivalence fallacy. When I pointed out that fallacy he didn't bother to reply [2]. > > You made the conscious choice to withdraw from the discussion > > immediately, so just like a person who abandons an election cycle and > > decides not to vote, you are leving the future of the matter in the > > hands of others. > > This is quite a disappointing argument. If this position were to be > accepted broadly within the project, it would suggest scorched-earth > last-man standing tactics -- just arguing until the other side runs > out of energy. If that was used to determine our forward strategy, > it'd result in a massive waste of energy, people feeling drained and > losing motivation to contribute, some people just deciding to leave > the project, and a myriad of other negative outcomes. This is how progress is achieved in all areas of society, from public debates, to trials, to elections. You can't just stand up and leave when the debate is only 25% complete, and then expect to win it. Either you care enough see it through, or you don't. Even in sports, and in video games, completing the match is the bare minimum to win. Unlike in a video game, the fact that you left the debate doesn't necessarily mean you lost, but you definitely did not win it. > In fact, occurrences of such behavior has already had such outcomes. That's your opinion, my opinion is the exact opposite. The reason people have left the project is because even though they care, and they stay for 100% of the race, if some bigwig who only said "I disagree" and left having only participated in 10% of the debate, the debate is over, always in favor of the incumbents. This is not a fair marketplace of ideas where the best ideas win, it's not a meritocracy, and discourages anyone who is not already part of the big club. > Rehashing the same arguments repeatedly damages the discourse within > the project as well as the project itself. There's no point in doing > so. All philosophers would disagree with you. There are some debates that have lasted for millennia and still continue today. Many arguments for free will are repeated exactly the same, but some are slightly different, and merit a new look. As long as we don't solve this debate, it will continue, and everyone that cares sees value in it. Similarly, until "git pull" does something sensible by default (which isn't the case now), these debates will continue, and there's value in them. Perhaps if other people didn't stand up and left in the middle of the debate in 2013 we would have solved the issue back then and we wouldn't be here. But here we are yet again, and if people continue ignoring arguments and leaving the debate before it's even 10% done, we will be here again in 2022, 2024, and probably more. Especially if they straight-up reject patches that according to themselves are 96% perfectly fine. Cheers. [1] https://lore.kernel.org/git/CAMP44s2XFQoda_PMULWha-rj9HhNfEddO5fikmswk9=AWN4RCw@mail.gmail.com/ [2] https://lore.kernel.org/git/xmqqpn3lbhxn.fsf@gitster.c.googlers.com/
On Tue, Jun 22, 2021 at 8:20 PM Elijah Newren <newren@gmail.com> wrote: > > On Tue, Jun 22, 2021 at 2:22 PM Alex Henrie <alexhenrie24@gmail.com> wrote: > > > > While > > we're on the subject, do you have any thoughts on what (if anything) > > more should be done before making the switch to aborting instead of > > merging with a warning in `git pull`? > > I think Junio already answered that over here: > https://lore.kernel.org/git/xmqq360h8286.fsf@gitster.c.googlers.com/ > (he discussed it multiple times in that thread, but hopefully that's a > good enough example). Wow, if I understand correctly from that message, Junio wouldn't mind making the switch right away. That's very encouraging. On Wed, Jun 23, 2021 at 12:19 PM Felipe Contreras <felipe.contreras@gmail.com> wrote: > > Similarly, until "git pull" does something sensible by default (which > isn't the case now), these debates will continue, and there's value in > them. At this point, I'm inclined to push for s/advise/die/ in pull.c in the next release, without a transitional period, just to end the argument over how to best explain the current awkward situation. (I'm sure there will be more arguments after that, but hopefully they won't be as tiresome.) -Alex
Alex Henrie wrote: > On Tue, Jun 22, 2021 at 8:20 PM Elijah Newren <newren@gmail.com> wrote: > > > > On Tue, Jun 22, 2021 at 2:22 PM Alex Henrie <alexhenrie24@gmail.com> wrote: > > > > > > While > > > we're on the subject, do you have any thoughts on what (if anything) > > > more should be done before making the switch to aborting instead of > > > merging with a warning in `git pull`? > > > > I think Junio already answered that over here: > > https://lore.kernel.org/git/xmqq360h8286.fsf@gitster.c.googlers.com/ > > (he discussed it multiple times in that thread, but hopefully that's a > > good enough example). > > Wow, if I understand correctly from that message, Junio wouldn't mind > making the switch right away. That's very encouraging. Junio has not paid enough attention to this topic. He is not aware of all the problems, and once he does he will very likely change his mind. > On Wed, Jun 23, 2021 at 12:19 PM Felipe Contreras > <felipe.contreras@gmail.com> wrote: > > > > Similarly, until "git pull" does something sensible by default (which > > isn't the case now), these debates will continue, and there's value in > > them. > > At this point, I'm inclined to push for s/advise/die/ in pull.c in the > next release, without a transitional period, just to end the argument > over how to best explain the current awkward situation. (I'm sure > there will be more arguments after that, but hopefully they won't be > as tiresome.) Give it a try. You will inevitably stumble upon all the problems I already fixed. In the meantime what's the problem with v2? Cheers. [1] https://lore.kernel.org/git/20210623004815.1807-1-felipe.contreras@gmail.com/
On Wed, Jun 23, 2021 at 11:55 PM Felipe Contreras <felipe.contreras@gmail.com> wrote: > > Alex Henrie wrote: > > On Wed, Jun 23, 2021 at 12:19 PM Felipe Contreras > > <felipe.contreras@gmail.com> wrote: > > > > > > Similarly, until "git pull" does something sensible by default (which > > > isn't the case now), these debates will continue, and there's value in > > > them. > > > > At this point, I'm inclined to push for s/advise/die/ in pull.c in the > > next release, without a transitional period, just to end the argument > > over how to best explain the current awkward situation. (I'm sure > > there will be more arguments after that, but hopefully they won't be > > as tiresome.) > > Give it a try. You will inevitably stumble upon all the problems I > already fixed. Patch sent. > In the meantime what's the problem with v2? I think that setting pull.rebase on a per-repository basis (instead of globally or per-invocation) makes for the easiest workflow in the majority of cases, so I would prefer to continue to recommend that to users primarily, but I don't have a strong opinion. -Alex
Alex Henrie wrote: > On Wed, Jun 23, 2021 at 11:55 PM Felipe Contreras > <felipe.contreras@gmail.com> wrote: > > > > Alex Henrie wrote: > > > On Wed, Jun 23, 2021 at 12:19 PM Felipe Contreras > > > <felipe.contreras@gmail.com> wrote: > > > > > > > > Similarly, until "git pull" does something sensible by default (which > > > > isn't the case now), these debates will continue, and there's value in > > > > them. > > > > > > At this point, I'm inclined to push for s/advise/die/ in pull.c in the > > > next release, without a transitional period, just to end the argument > > > over how to best explain the current awkward situation. (I'm sure > > > there will be more arguments after that, but hopefully they won't be > > > as tiresome.) > > > > Give it a try. You will inevitably stumble upon all the problems I > > already fixed. > > Patch sent. I sent a bunch of comments to the approach you sent. I think that's not all the issues, but it's been a while since the last time I took a good look at this, I feel I'm still missing one issue. > > In the meantime what's the problem with v2? > > I think that setting pull.rebase on a per-repository basis (instead of > globally or per-invocation) makes for the easiest workflow in the > majority of cases, so I would prefer to continue to recommend that to > users primarily, but I don't have a strong opinion. OK. What happens if you don't have the configuration in the particular repository you are using?
diff --git a/builtin/pull.c b/builtin/pull.c index 3e13f81084..48e25a5061 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -927,18 +927,20 @@ static int get_can_ff(struct object_id *orig_head, struct object_id *orig_merge_ static void show_advice_pull_non_ff(void) { - advise(_("Pulling without specifying how to reconcile divergent branches is\n" - "discouraged. You can squelch this message by running one of the following\n" - "commands sometime before your next pull:\n" + advise(_("Pulling without specifying how to reconcile divergent branches is discouraged;\n" + "you need to specify if you want a merge, or a rebase.\n" "\n" - " git config pull.rebase false # merge (the default strategy)\n" - " git config pull.rebase true # rebase\n" - " git config pull.ff only # fast-forward only\n" + " git pull --no-rebase # the default (merge)\n" + " git pull --rebase\n" "\n" - "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")); + "You can squelch this message by running one of the following commands:\n" + "\n" + " git config --global pull.rebase false # merge\n" + " git config --global pull.rebase true # rebase\n" + " git config --global pull.ff only # fast-forward only\n" + "\n" + "If unsure, run \"git pull --no-rebase\".\n" + "Read \"git pull --help\" for more information.")); } int cmd_pull(int argc, const char **argv, const char *prefix)
According to feedback from GitHub trainers [1], most newcomers don't understand what a rebase is. So in the default warning we want to provide our users with a command that does the most sensible thing, fixes the divergence, gets rid of the warning, with the minimum mental effort, and happens to be the default: git pull --no-rebase (later --merge) In addition, we don't want to start by recommending a permanent configuration, but a temporary solution so they start training their fingers and maybe learn how to do a rebase. So we start with the commands. Also, we need to be clear about what we mean by "specifying"; merge, or rebase. Moreover, thanks to the previous patch now "git pull --help" explains what a fast-forward is, let's mention that reference. And finally, use --global in the configuration commands like we did with push.default. [1] https://lore.kernel.org/git/20130909201751.GA14437@sigill.intra.peff.net/ Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> --- builtin/pull.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)