patch, 9 partsremote-helpers: fixes and cleanups

Hi,

Here's a bunch of cleanups mostly to synchronize remote-bzr and remote-hg.

One of these might conflict with a series already in pu, if so, the code here should be the prefered one.

Felipe Contreras (9):
  remote-bzr: trivial cleanups
  remote-hg: remove extra check
  remote-bzr: fix bad state issue
  remote-bzr: add support to push URLs
  remote-hg: use hashlib instead of hg sha1 util
  remote-bzr: store converted URL
  remote-hg: use python urlparse
  remote-bzr: tell bazaar to be quiet
  remote-bzr: strip extra newline

 contrib/remote-helpers/git-remote-bzr | 47 ++++++++++++++++++++++++++++++-----
 contrib/remote-helpers/git-remote-hg  | 17 ++++++-------
 2 files changed, 48 insertions(+), 16 deletions(-)

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Felipe Contreras wrote:

Why?  Is the new form faster than the older one?

What is this?  Whitespace?

How is this trivial?  You just removed one argument.

How on earth is this trivial?  It changes the entire meaning!

Whitespace?

I'm outraged by this. What kind of changes are you pushing to remote-hg? A "trivial cleanups" bundling miscellaneous changes, with no commit message? Why don't you just squash everything into one "miscellaneous changes" patch?

On Thu, Apr 25, 2013 at 1:19 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

has_key is deprecated.

s/blog/blob/

It's not an argument.

And nobody has noticed any problem.

Aka. trivial.

There are no miscellaneous changes other than the *possible* fix for deleted files. Which we don't know if it would actually fix anything, but as far as we know if it's a bug, nobody has seen it, and if it isn't, it's very unlikely that anybody is relying on the current behavior.

Plus the change seems to be obviously correct, as it comes from remote-hg, where somebody appeared to have found a bug.

That being said, I do remember writing an explanation for this in the commit message:

--
commit ca8c02dc7ea6395b1c864296f2500b718892fab8
Reflog: HEAD@{143} (Felipe Contreras <felipe.contreras@gmail.com>)
Reflog message: rebase -i (fixup): remote-bzr: trivial cleanups
Author: Felipe Contreras <felipe.contreras@gmail.com>
Date:   Tue Apr 23 18:29:49 2013 -0500

    remote-bzr: trivial cleanups

    Mostly from remote-hg. It's possible that there's a fix to delete files
    with spaces.

    Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Yeap, there it is. It was just squashed by mistake.

But I do not care that much really. The patch is good either way, if you don't like it, you go ahead and fix it, because I won't. I have 174 remote-helper related patches in my queue, and nobody benefits from rambling about a one liner that is obviously correct, not you, not me, not the users, not the developers.

Junio of course might disagree and drop this patch, but then he would need to deal with the fallout of possible conflicts. Or he can do the sensible thing and pick the commit message above. I have real issues to deal with, and I think the less-than-perfect commit messages in a *contrib* script that is extremely recent is a small price to pay for having nice and workable bzr and mercurial remote-helpers as soon as possible; our users would thank us, and in fact, they already are.

In my hurry to reorganize all the commits of my fourteen remote-helper branches, I squashed the commit message of a trivial fix into trivial cleanups. Big whooping deal.

Hyperbole much?

Cheers.

Felipe Contreras <felipe.contreras@gmail.com> writes:

You don't stick to the rules of this project, which have been pointed out already:

  The body should provide a meaningful commit message, which:

    . explains the problem the change tries to solve, iow, what is wrong
      with the current code without the change.

    . justifies the way the change solves the problem, iow, why the
      result with the change is better.

    . alternate solutions considered but discarded, if any.

Your project is moving too fast to put up with the established procedures in this community.

In fact you are pretty much holding us hostage with a "take it or keep it broken while causing more work" attitude:

You did not respond well to reviews and criticism. Even the constructive fine-let's-do-the-work-for-him kind that Peff offered.

And on top of that, remote helpers are written against an interface that was designed to allow working with external programs.

So why is this in git.git?

Why should we take any more contrib additions from you?

On Thu, Apr 25, 2013 at 3:30 PM, Thomas Rast <trast@inf.ethz.ch> wrote:

The rules of the contrib area are different from the ones of the rest of the project.

That's one of the reasons it's in the contrib area.

I'm the maintainer of this code, so it's my call. If Junio has a problem with that, I would gladly take my code somewhere else. I doubt that's in the best interest of anyone.

But if the problem is this particular patch (reaally?), Junio could just drop this particular patch. Are you seriously suggesting that the whole contrib/remote-helpers should be dropped because this patch introduces a one-liner fix without mentioning it in the commit message? Really? I haven't seen anybody complain about *any* of the other patches where I "held the project hostage" and refused to fix the commit message or change the patch.

Other than this instance, show me where exactly did I do that.

Define "respond well". If your idea to "respond well" is to say "Yes sir!" to every criticism, then no, I didn't. OTOH, if it's to reply and address the issues with objective reasoning and an open mind, I did.

I don't understand this notion that every review criticism is valid and correct. They are not, and it's OK to point that out.. really. If they turn to be valid and correct, the reviewer can surely counter-argue and substantiate his/her claims.

And I don't recall Peff ever doing this "constructive fine-let's-do-the-work-for-him" on any contrib/remote-helpers stuff.

Because it's good for the users.

If you are seriously suggesting to drop contrib/remote-helpers, I suggest that 1) don't do it in the review thread of a trivial patch 2) start a new thread where you point multiple instances where the maintainer of the code (me) failed to respond correctly to criticism (of remote-helpers's code), 3) show how this affects negatively the project, and 4) ask for new maintainers if the job of the current one is not deemed up-to-par, and only if no maintainer steps up, drop the code.

Cheers.

Felipe Contreras <felipe.contreras@gmail.com> writes:

Yes and no. 

A contrib/ material may not be held to the same high standard, but that does not mean a contrib/ area maintainer has a blank check to do anything there.

It would be pretty obvious to people observing what happens in the area after a while, if the quality standard the area maintainer enforces is too out of whack, and at that point the area maintainer deserves to be ridiculed ;-)

I do not think Thomas was talking specific about contrib/ material but your interaction in general with other developers.

Cf. http://thread.gmane.org/gmane.comp.version-control.git/220427/focus=220891

FWIW, I thought "that person was me" response from him was more than reasonable, and I still do.

On Thu, Apr 25, 2013 at 4:37 PM, Junio C Hamano <gitster@pobox.com> wrote:

Nor did I claim I had one.

Of course, but the claim the rules are different still stands.

Yeah but how is that relevant in this context? We are talking about a particular patch of remote-bzr. And he immediately used that claim as ammunition to suggest the whole remote-helpers should be dropped. It does not follow.

Any suggestion to drop remote-helpers should use facts and arguments regarding remote-helpers.

Felipe Contreras <felipe.contreras@gmail.com> writes:

Three random points.

 * For this particular patch [1/9], especially because this would
   land close to the corresponding remote-hg fixes (e.g. "has_key is
   deprecated"), I think it is sufficient to say "port fixes from
   corresponding remote-hg patches" (you said it in 0/9 and didn't
   say it in 1/9, though) without going into individual details.
   Anybody who wonders what these changes were about will have a
   clue to check contemporary patches to remote-hg that way.

 * You may want to hold onto those 174 patches and polish their
   explanation up to save the list audiences' time by avoiding this
   kind of useless "why no explanation" exchanges.

 * If you do not want to keep a readable history, it would mean that
   nobody but you will fix problems discovered in the future in
   remote-hg, and there is no point carrying it in my tree for other
   Git developers to look at it.  The users are better off getting
   them from your tree and that will make it clear for them whom to
   ask help/fix for when they hit a snag.

A much more sensible thing in such a case for me to do actually is to drop the whole thing. I do not want to do that unless necessary.

I do not share this view at all. The users survived without it long enough; they can wait for a well maintained version. On the other hand, shipping something that will not be maintainable is not the way to help end users. It is being irresponsive to them.

Helping other developers understand your code is a way to ensure that your code that would help users will be kept maintained. I do not agree with Ram at all when he says that developers are more important than users, and I agree with you that the project exists for users, and not for developers. But you need to help your fellow developers anyway by spending effort to keep your history readable, in order to help them help the users.

Do not take the "users matter" mantra to the extreme. You need other developers to put users first.

On Thu, Apr 25, 2013 at 3:36 PM, Junio C Hamano <gitster@pobox.com> wrote:

I don't see the value of pointing that out in the particular commit, since you are the only one that would do anything with that information, and it seems the message came across.

If there's any issues with that, just drop the patch, and if there's issues with the rest of the series, just drop them. I'll resend when the stuff is merged to master.

That's exactly what I've been doing.

You are extrapolating from this particular patch, which I already admitted I made a mistake, and it's not really important in any way.

The history *is* readable. If anybody has any problems with the commit messages, the place to mention such problems is IN THE PATCH REVIEW. Nobody has done that, because either nobody has any problems, or they are not interested. Either way, there's nothing I can do about it.

*This* patch is an exception, and I'm not willing to waste time on this extremely trivial patch. Drop it.

You want to drop the whole series because of a cleanup patch with a less-than-perfect commit message? Even though there quite likely won't be any conflicts if you drop the single patch. Fine, drop the whole series.

Are you saying that because *ONE PATCH*, introduces a fix without mentioning it in the commit message, *THE WHOLE* project becomes unmaintainable?

If not, then why are we discussing about something that is not happening?

And I am. Because I made a mistake in this patch doesn't mean the same happened in all the patches.

I am helping my fellow developers by replying to the comments they make when I send the patches for review. Unfortunately, the only developer other than you that has made any comment at all, Ramkumar Ramachandra, did so in a bellicose tone, but I replied to all his comments either way, which where invalid. The only comment where he is right and I acknowledged making a small mistake, is trivial, does not cause any issues, and can be easily dropped.

No, I don't. It would be nice, yes, but not necessary.

Now, let's drop this pointless discussion and deal with the actual issue. What do you want to do?

1) Drop this patch
2) Drop the whole series
3) I reroll without the change that was not described

Anything else, I'm not interested in doing. There's tasks with actual value to do.

Cheers.

Felipe Contreras <felipe.contreras@gmail.com> writes:

Just in case you missed it, the first in the three-random-points was "I personally think 1/9 that does not say anything about the minute and irrelevant details Ram kibitzed about is fine". So "Drop this patch" is not something on the table in the first place.

 * After seeing that this change is a copy from recent remote-hg
   changes, a revier who did a little homework would easily find a
   change around has_key in recent patches.

 * A reviewer who did a little homework would know by reading a bit
   beyond the patch context to see that nobody uses "bmarks".

 * A reviewer who wondered how the two lines are different can stop
   staring at the screen, take a walk and come back with refreshed
   eyes to spot the difference between blog and blob very easily.

For these reasons, I personally do not think it is unreasonable to throw comments like the ones on "has_key", "global bmarks", and "blog vs blob" into "too obvious, not even deserve to be responded" bin.

Having said that, I am more worried about wasting everybody's time (and this includes your time) with the impedance mismatch between you and the rest of us.

Our standard for explaining the change (either in the log or in the comment) is to err on the descriptive side to be helpful even to people new to the codebase. We do not require or encourage to state the obvious. The issue is the definition of "obviousness" varies even among the rest of us and even for a single person depending on how familiar that person is with the area of the code in question. But the divide between you (alone) and the rest of us seems to be far more vast than differences among the people other than you.

Especially the criteria I used in the above example for "bmarks" need to be used carefully. If a reviewer needs to follow a very deep callchain to convince himself why a change does not break things, it is no longer obvious and deserves to be explained.

So I dunno. If you are not willing to change your ways and try to be more descriptive to help others to understand what you are doing, there is nothing I can do to help you.

On Thu, Apr 25, 2013 at 5:01 PM, Junio C Hamano <gitster@pobox.com> wrote:

You are missing my point, this is *ONE INSTANCE*. Show me another instance where a reviewer complained about the lack of a descriptive commit messages on *remote-helpers*.

So if I'm not willing to describe every little trivial cleanup change I do, what should I do then? Avoid those trivial changes?

If your true purpose of having descriptive commit messages is to improve maintainability, then actually doing these cleanups should have priority over a descriptive commit message, because doing the cleanups improves the maintainability even without a detailed description.

Clearly, your reasoning is incomplete.

I'm willing to change my ways when there's reason to change my ways, and so far, nobody has provided any evidence that my commit messages are indeed lacking, only *opinions*.

Other people are perfectly fine with them: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/log/?qt=author&q=felipe.contreras

And the only reason we are wasting time, is that *you* make us waste time. Any sensible reviewer would be context aware, notice that this is a contrib patch, and focus on behavioral changes, notice the mistake I made, and point that *one* of the changes was changing the behavior, at which point I would agree and reroll either without that change, or with the change in a separate commit (which I don't want to do right now). The maintainer (you), wouldn't even have to reply at all.

But the reviewer failed to do so, and other contributors went even further, so the ball is in now in your court. IMO a sensible maintainer would simply say "Guys, stay on topic, what do we do with this patch?", but no, you allow people to suggest that not only the whole series, but the whole sub-project be dropped, and to do so with totally unrelated facts, and generalizing from *ONE INSTANCE* in the actual sub-project, and generally from ad hominem arguments.

This doesn't help anybody.

Show me a systemic problem with the commit messages *in remote-helpers*, and then perhaps it would be worth to start *a new thread* to discuss them, but nobody has done so. We are still talking about a *single patch*.

And if you really really don't like the patch, say "do X, or I drop the patch", or the series, and there would be no need for other reviewers to waste their time (if their comments were truly valid and correct, which they are not). There's no need to say anything more. And even if the reviewers were correct in their comments, allowing suggestions such as that the whole sub-project should be dropped because of one patch is going to waste people's times, no matter what.

Cheers.

Felipe Contreras <felipe.contreras@gmail.com> writes:

You are the one who is missing the point. My message was about your patches to _any_ part of our system, not limited to remote helpers.

On Thu, Apr 25, 2013 at 6:11 PM, Junio C Hamano <gitster@pobox.com> wrote:

I still see "Re: [PATCH 1/9] remote-bzr: trivial cleanups", if we are talking about something else, let's do so and be clear in the subject, but I don't see what this has to do with this trivial patch, the series, or even remote-bzr in general (for which nobody has complained bout commit messages before).

Felipe Contreras wrote:

Personally, I think it is the job of the submitter to provide a really helpful commit message and widen his review audience. If I'm hitting the git mailing list with my patches, I try to make sure that nearly everyone on the list can understand what I've done and potentially review it. Why else would I want to hit their inboxes with my patches?

Here's my solution to the problem: maintain your project outside git.git and merge changes in every couple of months or so with a simple email containing a pull URL, addressing Junio. If Junio trusts you enough to put the changes you send into contrib/ after a cursory glance, we're done. Start a separate mailing list for your project/ accept GitHub pull requests via which contributors can send you changes. No more fuss or drama on the git list about this. You can be as stubborn as you want, and we go back to our lives. Everyone wins.

If you want to submit patches to other parts of git, you seriously need to change your ways. Let's deal with that problem when it arises next.

On Fri, Apr 26, 2013 at 7:19 AM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

If you don't understand the reasoning and history behind remote-bzr, you might be doing a disservice to everyone by commenting at all.

Bazaar is a dead project, and there are *real* users suffering as we speak, bound to eternal SCM torment by evil dictators and political non-speak. Even the worst of remote-bzr patches are a thousand times better than what you see in bzr code itself.

To give you some perspective, one commit broke a branch in the emacs project, and ever since then people are not able to clone that branch. This bug has been known for years, and nobody fixes it. Every time anybody tries to clone that branch, they need a special sequence of commands.

They *need* something like remote-bzr to escape the horrendities of bzr, and all you are doing complaining about a sneaked fix is a disservice to everyone. Yes, doing such a thing on git.c would not be particularly great, but wouldn't be horrific either, fortunately we are not doing that!

Answer me, do you use bzr? No? Do you use remote-bzr? No? Then how in hell could you possibly have any contextual information to make even a guess as to what would be the impact of sneaking such a small fix? You can't.

But why are we even speaking about this nonsense? This patch has been dropped. You want to review something, go review PATCH v2 1/9. Stop arguing about stubbornness and hypotheticals, when there's actual code to review.

What is your objective, do you want to help this project move forward or not?

Felipe Contreras wrote:

Forward, please.

I want a solution to this persistent problem of conflict though. And I presented one in my previous email:

Here's my solution to the problem: maintain your project outside git.git and merge changes in every couple of months or so with a simple email containing a pull URL, addressing Junio. If Junio trusts you enough to put the changes you send into contrib/ after a cursory glance, we're done. Start a separate mailing list for your project/ accept GitHub pull requests via which contributors can send you changes. No more fuss or drama on the git list about this. You can be as stubborn as you want, and we go back to our lives. Everyone wins.

I'll probably even contribute small patches once in a while.

On Fri, Apr 26, 2013 at 1:53 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

I already maintain my own clone outside of git.git[1], and I do already accept pull requests[2], and people have sent me patches directly. The stuff I send to the git mailing list is what I think is ready for merging. But there's only so much stuff I can catch.

We all benefit from these patches being reviewed in the git mailing list, nobody has claimed otherwise. You are making the error of assuming that your review was actionable, that I should have done something, fix the commit message I suppose, but I don't think that's important.

In contrast, this is how a constructive, valid and helpful review looks like:

http://article.gmane.org/gmane.comp.version-control.git/220034

Junio caught a problem I didn't see, I accepted the valid feedback, and I resent with a fix. We all benefit from such interactions, both users, and developers. What's wrong with that?

You just got angry that your review didn't turn out to be helpful, is that it? Why do you want to steal helpful review from the users of remote-{bzr,hg}? If that's not the case, please stop doing that. All review is welcome, not all review should be acted upon.

Cheers.

[1] https://github.com/felipec/git [2] https://github.com/felipec/git/pulls?direction=desc&page=1&sort=created&state=closed

Felipe Contreras wrote:

What I'm saying is that you can get more eyes. A lot more eyes. If you just write a proper commit message!

Why are you hitting everyone's inboxes with such cryptic patches that
require either:
1. The reviewer to trust what you've done and move on.
2. The reviewer to do a lot of digging before the patch becomes
accessible to her.

I'm not angry about anything, or trying to steal anything.

What happened: New email. Felipe's remote-hg fixes. Okay, let's look at this. Part 1. What?! [I wrote down what I was thinking as I was reading the email]

This is where you _should_ apply reason: justify everything you've done in the patch in your commit message. Why are you so stubborn about not wanting to change your ways despite so many people telling you? Is it your pride*?

* Yes, I noticed that you have a huge ego.  I consider it an undesirable trait.

On Fri, Apr 26, 2013 at 2:56 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

It is a trivial obvious patch that changes very few lines of code, it doesn't need more eyes in my opinion. For the patches that do need eyes I do send descriptive commit messages. And some patches that I don't think need eyes, but I think I could do wrong, I also write descriptive messages.

So far, it looks like I was right in thinking this patch didn't need more eyes. And I think my original commit description, which I deleted by mistake was more than enough "Mostly from remote-hg. It's possible that there's a fix to delete files with spaces". Because so far, nobody has pointed out any actual issue, and it's not clear if this patch would benefit from even more eyes... probably won't.

Because there's no other way to get the changes into git.git. If you wan't I can add "DO NOT REVIEW" in the title, but I think "trivial cleanups" pretty much sums that what I feel, and actually I wouldn't want people to _not_ review the patches, but rather to understand that I think they are trivial, and shouldn't worry too much about them.

Good, so I'll keep sending the patches, because our users benefit from the review.

Write what you see, not what you feel. Your questions about the code are fine, but making assumptions about what remote-bzr users must be suffering by not having more descriptive commit messages are not. You also assumed that I wanted to send that commit message, when that was not true, I removed a chunk by mistake.

In general, you shouldn't make assumptions.

Stop asking these questions, I thought you already agreed this patch was not worth discussing about. If you see *any other* patch that doesn't have a good enough commit message, reply _there_. And if you do want to pursue these questions irrespective of this patch, start a new thread.

I don't think so, but even if I did, it doesn't matter, all that matters is that my arguments are sound and valid, you should concentrate on the ball, not on the man. The fact that I believe my arguments are valid and sound doesn't make me egotistic, it might be that they are actually valid and sound, and I'm simply assessing them correctly. I of course I'm willing to admit otherwise, based on evidence, and reason.

Cheers.

Felipe Contreras <felipe.contreras@gmail.com> writes:

Just for the record, a patch sent to the list which nobody bothered to read does not really count as reviewed.

You can either

 (1) pace yourself when people are otherwise busy; or

 (2) send them anyway but not claim "this was sent to the list two
     weeks ago, nobody complained, so it must be perfect" when it is
     not picked up after a few weeks.

Often (1) is a better strategy, as people who wanted to review but otherwise were busy tend to declare patch bankruptcy after their busy period ends.

Also, a reason that a patch goes uncommented is when it is difficult to judge. A patch with code change without sufficient explanation behind the motivation to justify the change, a reviewer finds it much harder to convince himself that the patch is a good change, and it also is much harder to find which part of the change is wrong and offer improvements, compared to a patch with the same change that is justified properly.

On Fri, Apr 26, 2013 at 5:10 PM, Junio C Hamano <gitster@pobox.com> wrote:

No, but I did my part, which is sending them for review.

I would, if there was a reason to.

When have I ever done that?

Not for remote-{bzr,hg}. I've yet to see anybody claim they would review the patches thoroughly, if only they were given time. I've yet to see anybody claim they would review the patches thoroughly under any circumstance at all. And by that I mean the patches that really would benefit from reviewing.

Yes, but is that the case *HERE*? And no, single line changes that are trivial and obvious don't count. Show me an important patch that surely would benefit from reviewing that doesn't have sufficient explanation. Show me an important patch that anybody is not convinced is a good patch. In the remote-{hg,bzr} context.

If there isn't any, I don't see why remote-{bzr,hg} should slow down.

For this patch, I don't care one iota.

Cheers.

Felipe Contreras wrote:

Felipe, I'm trying to help. If you think my review lacked context, you can write me a paragraph/ link me to an email and I will read it.

It's not reviewers and submitters "attacking" each other. It's helping out other rational people in the community because you care for their reviews. Don't practice exclusivity and label some people as "not eligible to review". That's not a good way to develop.

My God.  This is horror story.

No Felipe, I don't use bzr.

Felipe Contreras wrote:

I've never wanted to pick fights with anyone, and I don't foresee having a desire to do so in the future. I was just saying what was on my mind, which is along the lines of: you have written this patch with the attitude "I know what I'm doing, my users will benefit, and nobody else is going to look at this patch anyway"; I'm worried about what your other patches look like if this is your attitude towards development. Junio is harping about the same thing: the impedance mismatch between you and the rest of us.

That's what I've been trying to say over and over again: _why_ are people not reviewing your patches?

0. Because nobody has any problems with them.

1. Because nobody on the git list cares about remote-hg.

2. Because you're stubborn as a mule, and the resulting thread often
results in long-winded discussions like this one (which wastes
everyone's time).  Therefore, the potential reviewer's reasoning is
that their review time is better spent elsewhere, where their review
is actually appreciated.

Hint: it's not (0).

If you're claiming that (1) is the case, then why are you posting to the git list and hitting everyone's inboxes? Maintain your project outside git.

I'm claiming that it's (2).  In which case, it's you who needs changing.

You want a formal mathematical proof? We operate on opinions, and freeze what we think we all agree with into "community guidelines".

So you're now claiming that we're the ones at fault (Peff, Thomas, Junio, and me, among others). Okay, so why are you forcing your changes and opinions down our throats? You're in the wrong community: join a community of people who are more like you (or start your own project), and stop wasting our time.

Junio C Hamano wrote:

On this.

If Peff were to suddenly stop working on git one day (because he was frustrated with the community/ development practices), we'd all lose a lot more than if one random end-user switches to using hg for his tiny personal projects. I'm _not_ claiming that there's a split between users and users that are developers (we have one mailing list for everyone, and I like that). What I'm claiming is that we cannot (and should not) pay equal attention to every user of git. Some users are more important than others. Again, that does _not_ mean that we push a change that benefits one important user but breaks everyone else's setup.

Ofcourse the project exists for its users; we're not doing research. However, we don't all have to write tutorials to keep in touch with end-users who are completely detached from the development process (our time is better spent elsewhere), or even have an end-user-friendly bug tracker (where the SNR is very low). We don't have to consciously reach out to people we're not connected to directly: if we're all sufficiently connected to the real world, the itches/ bugs worth working on will always find their way to us. We live in a connected world.

Yes, I know. You're going to respond to this email arguing about why you're Right and why I (and everyone else) is Wrong, either by quoting what Linus (TM) said and twisting it to mean what you want, belaboring over what you've already said, or something similar.

I've given up on you, and I suspect a lot of other people have too.

On Fri, Apr 26, 2013 at 4:32 AM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

That is an assumption, it's wrong, and it's antagonistic. There's no need for that.

More assumptions and hypotheticals. Why don't we limit ourselves to facts and reality?

This is the false dichotomy fallacy, why does it have to be only one of these reasons? Couldn't it be a mixture of them? Maybe most people don't care about remote-{bzr,hg}, maybe for the ones that do, most don't see any problems in the patches, and maybe the ones that do see problems in the patches don't bring them up, because of various reasons, like for example, they don't see them as major, and would rather fix them themselves later after investigation if they are indeed import problems, or maybe they just don't have time to engage in discussions.

Yes, there's a possibility that my stubbornness is a factor, and given their possible lack of time, and possible lack of interested, they choose to not engage.

But to claim that *everyone* is in (2), and that there are no other factors that made them land in (2) but my stubbornness is disingenuous at best.

What would you think of a scientist that says, oh, "I'm not going to review that paper, because each time I do that, the reviewee defends it, and we end up with long-winded discussions". This scientist doesn't have the right spirit.

If you submit a review comment, you should be prepared to do defend it, just like you do when you submit a patch. And you should be prepared to accept that your patch has mistakes, just like you should be prepared to accept that your review has mistakes. In this view, stubbornness is a good thing, because it brings the best in the reviewer, and in the reviewee, and in the end everyone benefits by trying to achieve the higher quality possible. Just like stubbornness is a good thing in science, if both reviwer and reviewee fight for their point of view, only the best science wins, and we all benefit.

This is all of course if the stubbornness is warranted, but you haven't claimed that it wasn't, simply that I was stubborn. Well, so what? Isn't that good?

Show me where I was unreasonable, show me where I was wrong, not where I was stubborn. One can be stubborn and be right, in fact, when one is right, it's when one has all the more right to be stubborn.

If you are not prepared to defend your review, so are others, why to you blame that on me? If you were right, you would be shown to be right. Period.

No, we operate in evidence and reason, *not* opinion. Any reasonable person would say "well, I *think* this commit message needs more description, but I don't *know*, I don't have *evidence* for it, so I'm not going to fight to the death, as if I had".

Any reasonable person would know the difference between an opinion, and an objective fact. And react accordingly when another person attacks an opinion, which is not a big deal, and when an objective fact is attacked, which is a big deal.

Where am I doing that?

First of all, lets not act like bitching girlfriends arguing about who didn't throw the trash three years ago. We are talking about *remote-bzr*, in fact, the subject is "[PATCH 1/9] remote-bzr: trivial cleanups". *NONE* of this discussion is relevant to that patch.

If we go one step above, to remote-helpers in general, nobody has commented *ANYTHING* negative on those patches, you are the first to do so. So how exactly did I managed to shove 71 patches down you throat if everybody complained all the way?

Ultimately the decision to merge or not to merge comes to Junio, if you don't like his decision, go complain to him, but I would be prepared with points in time where people complained about these patches, and there are no complains, so you have no ammunition at all whatsoever.

If you are talking about something else, FFS, change the subject line. If you want to argue about who didn't take out the trash three years ago, fine, but lets do so clearly in another thread, not this one about a *single trivial patch*.

And this is how communities die. When everybody thinks the same, and everyone who thinks differently is displaced. A monoculture, a place full of yes-men where nobody criticizes anybody, a circlejerk where everyone palms the back of everyone else. Eventually things go south, and nobody around you understands why.

Diversity in a community is healthy. If you don't like people who think differently, *you* have a problem. If you don't like standing up and defending your ideas, *you* have a problem. If you don't like discussing on the basis of evidence and reason, *you* have a problem.

Cheers.

[completely off-topic; don't worry, we're just having a friendly chat]

Felipe Contreras wrote:

Felipe, there are some things that are worth arguing about for a long time (like the new revision spec I'm proposing in [1]), and others that are not.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/222248/focus=222526

Don't you think you're taking reason to an extreme here? Reason is a tool that I use when I want. I don't want reason when I'm browsing Google Art Project or listening to Gentle Giant. Arguments like "is this commit message large enough?" are really not worth the time and effort.

I have no desire to "attack" you, Felipe. I respect you as a more experienced developer than myself, and am trying to offer constructive criticism.

I don't have an ego (or consider myself important to the community). Whatever will happen will happen, with or without me.

Diversity is certainly healthy, and I it would be nice to have you in the community. We just have to find a way to keep the conflict down.

Ramkumar Ramachandra wrote:

After all, what are we asking for? Better commit messages. Why are you making such a big deal out of it? You want diversity in "length/ form of commit messages"?

On Fri, Apr 26, 2013 at 2:30 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

I agree, and I have discussed about issues with diff in the past, unfortunately didn't reach any conclusion. I've tried to follow that thread, but I don't really know what is actually being proposed anymore.

If you are so keen in receiving feedback from your fellow developers, you should eventually send an email summarizing the issues and the proposal for everyone to understand.

In contrast, I take it you agree this trivial patch is not worth discussing, which I agreed in my first reply.

Reason is not a tool for appreciating art, reason is a tool for discovering truth, and if when arguing you are not interested in what is actually true, I'm not interested in arguing with you.

I appreciate your criticism, but that doesn't mean I must agree with it. And if I do agree, that doesn't mean I must act upon it.

A fine way to start is to not rattle away in trivial inconsequential patches.

Cheers.

Felipe Contreras wrote:

I have something from Linus (TM) this time :)

https://lkml.org/lkml/2004/12/20/255

On Fri, Apr 26, 2013 at 3:03 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

I happen to agree with that, specially in the context of the Linux kernel, but I don't see how that applies here. Linus is talking about trivial patches from an entry-level developer, who has much to learn, and this is one the best ways to do that.

But in particular, he is talking about the fact that prominent kernel developers don't spend too much time on these trivial patches from these entry-level developers, and that can be frustrating for these entry-level developers, which can be problematic.

Nothing at all related to what we are facing here.

Felipe Contreras wrote:

Thanks.  I'll do that in the morning.

There is no great truth to be discovered by arguing about the length of commit messages, Felipe. There are some "guidelines" or "axioms" upon which we build reason. If you want to argue till everything breaks down to Peano's Axioms, do Foundations of mathematics or Analytical philosophy. From personal experience, it's much more satisfying than arguing with other humans (who aren't exact creatures).

Why not? Am I being unreasonable in asking you to justify your changes, so I can understand what you've done with one quick reading?

On Fri, Apr 26, 2013 at 3:28 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

And based on what do you build these guidelines and axioms if not truth? Do you ask a computer to throw a number randomly from 0 to infinite and that shall be the new axiom for what the perfect number of words a commit message should have?

No, you determine that based on experience, and convenience. You find a number that convenient to write, not hundreds of pages, and a number that would help the reader if the patch in case a bug in the changes is found on a later time, and that would help reviewers of code find issues, and understand it. But most importantly, the number depends on the complexity of the code changes. Note that I'm not saying on size, because even one-liners can be extremely complex.

It's not arbitrary.

I do not want to argue the fundamentals of logic and reason, but unfortunately most people don't have a strong grasp on them.

So let me simply; truth matters, and how we find truth mattes. Which is why the degree of certainty we have on certain facts matters; you shouldn't act the same about a claim you are 10% sure it's true, than with a claim you are 90% sure.

If you are 100% sure my commit messages are too short, then there's no point in arguing with you. Nor if you think it doesn't matter if it's 90%, or 50%, or even 0%. Because it's an opinion, and an opinion doesn't need any facts, or certainty, it just needs a person to hold it, whatever unreasonable or unlikely it is.

I did justify everything, I just didn't act the way you wanted. I didn't immediately resend the series with a full description of the changes, because the changes, as I described before, are trivial. I simply dropped the change you had a problem with, and moved on. It's perfectly reasonable.

Cheers.

On Fri, Apr 26, 2013 at 4:32 AM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

Yeah but that's not happening is it? This is yet another hypothetical. Last I checked Peff never threatened to leave the project. Did I miss a memo?

The last time somebody announced he was going to leave the project we did what was reasonable; investigate the reasons. And that's what we would do if Peff threatened to leave.

But fine, lets assume there's a hypothetical Peff, with hypothetical reasons to leave the project...

The importance of users changes all the time. The 15 year old kid in Sao Paulo might not be important today, but he might be the single most important contributor ten years from now. Hell, he might even replace Junio as the maintainer.

Who are you to decide which users are important, and which are not?

Maybe we should, and maybe we would see then some areas of improvement. In fact, I have done so, and I do see lots of areas of improvement in git's UI.

I agree that there are more important areas, or rather, more fun to work with, but the fact that most git developers don't pay too much attention to the pain of newcomers shows, and it's a very common criticism of git; it's difficult to learn, it doesn't have a consistent UI, many commands don't make sense. And I happen to agree with that claim, but it's not an easy problem to solve, specially when you care about *all* users, both old and new, which we do.

We should keep in mind the problems in git's UI for newcomers. There's no reason no to.

Nobody is claiming we need a bug tracker, there's no point in arguing about that. The rate at which we fix bugs or our tracking of them is not a problem.

Where did I twist anything? You can see Linus talk himself: http://www.youtube.com/watch?v=kzKzUBLEzwk

Point me exactly where does he say some users are more important than others, in fact, he is saying the opposite, the amount of people that needed the Linux version compatibility flag was really really small, yet they did it, why? Because *all* users matter. Not that it matters what Linus says, what matters is that it's right; the moment you start balkanizing your user base, the moment you start giving some them reason to fork the project. When was the last time Linux was forked? GNOME did exactly that, they said; you, users over there, we don't care about you anymore, what did they do? Fork. They lost so many users they had to revert their decision.

Should we willingly and knowingly neglect some git user-base? No, why would you want them to fork? In a way, git's UI has been so bad, that some kind-of-forks have happened, that tells us something; the UI needs some love, fortunately none of those forks worked, which tells us something too; it's not too atrocious.

That's not to say we shouldn't fix the UI, we should, in a way that everyone's happy, which is hard, but we will do it, eventually.

Cheers.

Felipe Contreras wrote:

Yes, they do.  Did I say that they don't change?

Yes, I watched the talk when you posted the link last time. And yes, I learnt something.

No, we should never neglect. I believe in including everyone. In fact I take it to an extreme: on many instances, I have pointed out what I want specifically, and asked for a configuration option if it's not necessarily a sane default. Git is a toolkit, and should be loaded with features that even a few users want.

On this, I think the way forward is complete-implicit'ness via configuration variables. I recently wrote remote.pushdefault to simply 'git push', and proposed 'git push +ref1 ref2 ref3' to automatically push to the correct pushdefaults (but that proposal was rejected).

On Fri, Apr 26, 2013 at 3:17 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

But you implied we shouldn't care about Thiago (our hypothetical future overlord), because he is among the users we should't care for (right now).

Indeed, I learned about that, and I tried to use it, but I think there's a lot that is missing, and I don't know myself what would be ideal. I'm starting to think that a branch should have two upstreams; one that is used for rebasing, and another that is used for pushing. But I'm not sure.

Eventually, I would like to do 'git push' and I would push different branches to different repositories in different destination branches in a way that requires multiple commands right now 'git push github fc/remote-old/hg:fc/remote/hg', 'git push --prune backup refs/heads/*:refs/heads/* refs/tags/*:refs/tags/*'. And to figure things out I'm also helping; I added the --prune option to push, and I added color to visualize upstream branches in 'git branch'.

But I don't think any of those are as important as having a proper 'git stage' command, and getting rid of --cached and --index, which will be a huge effort, but would pay even bigger dividends. Step by step.

Cheers.

On 04/25/2013 08:19 PM, Ramkumar Ramachandra wrote:

I think the has_key() method is "deprecated" in modern python, and the 'key in dict' usage is more idiomatic.

Typofix: s/blog/blob/

Maybe bmarks was no longer used there as a global variable (left-over from previous patches?), so there is no longer any need to declare it global.

Indeed, that should best go in a separate path with a proper explanation in the commit message.

Isn't that obvious?

I have no opinion on this, so I won't comment.

Regard,
  Stefano

On Thu, Apr 25, 2013 at 2:29 PM, Stefano Lattarini <stefano.lattarini@gmail.com> wrote:

Even more, it never was used, it was a mistake carried when copying this method from remote-hg; we don't have bookmarks in bazaar. And bmarks wasn't even used in this method in remote-hg either =/

But it would be obvious that it was not used once one ran the tests and they passed, which they do.

Cheers.

Not needed since we use xrange ourselves.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-hg | 4 ----
 1 file changed, 4 deletions(-)

Felipe Contreras wrote:

I'm surprised these four lines were even there in a previous revision.
 Again, you changed the meaning: if xrange() returns an empty range,
you must return, by extension.

On Thu, Apr 25, 2013 at 1:23 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

I'm not going to go back in history, but we were not always using xrange, but the mercurial API helper, which was dead slow, and in the end did basically an xrange().

Carried from remote-hg.

The problem reportedly happened after doing a push that fails, the abort causes the state of remote-hg to go bad, this happens because remote-hg's marks are not stored, but 'git fast-export' marks are.

Ensure that the marks are _always_ stored.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 6 ++++++
 1 file changed, 6 insertions(+)

Just like in remote-hg.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

To be in sync with remote-bzr.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-hg | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Felipe Contreras wrote:

Huh? Why do you have to be in sync with remote-bzr? Are you sharing code between remote-hg and remote-bzr?

Did you eve bother justifying this change with a line in the commit message? How is the new form different from the old form?

On Thu, Apr 25, 2013 at 1:25 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

We don't have to.

Why would it be any difference? It's a hex version of the SHA-1 digest. It would be the same in every language and every tool.

And a bit of context: historically the reason I started remote-bzr was to show that we didn't need the *huge* infrastructure that is sitting git_remote_helpers, which is nothing compared to what was prepared to be merged for msysgit's remote-hg. I wrote it as a proof-of-concept to show we didn't need a framework, and if we do, it would only be clear after having _two_ remote helpers, which we now do. It might make sense to refactor the common parts into a framework later on, so having them in sync as much as it's reasonably possible makes sense.

But if even if it wasn't, there's nothing wrong with this patch. Also, who knows, maybe old versions of mercurial don't have util.sha1(), or maybe newer ones will move it, who knows.

Cheers.

Mercurial might convert the URL to something more appropriate, like an absolute path. Lets store that instead of the original URL, which won't work from a different working directory if it's relative.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

It's simpler, and we don't need to depend on certain Mercurial versions.

Also, now we don't update the URL if 'file://' is not present.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-hg | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Otherwise we get notification, progress bars, and what not.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 3 +++
 1 file changed, 3 insertions(+)

It's added by fast-export, the user didn't type it.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 4 ++++
 1 file changed, 4 insertions(+)