Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Josh Triplettwrites: >> But submission is less important than review. And for review it is >> usually better (except gigantic series) to have patch text for review >> with the review. > > Agreed. However, submission typically requires more work than review, > because the patch text must remain applicable. For review, as long as > the email client you use to respond doesn't do something horrible like > *re-wrap* the quoted patch text, the result will work as a review. Yup. That is why we say "please send patch inline; when asked to send it as an attachment, please do so". -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On 10 August 2016 at 02:55, Josh Triplettwrote: > On Tue, Aug 09, 2016 at 06:28:00PM +, Eric Wong wrote: >> Some of these problems I hope public-inbox (or something like >> it) can fix and turn the tide towards email, again. > > This really seems like the dichotomy that drives people towards central > services like GitHub or GitLab. We need an alternative that doesn't > involve email, or at the very least, doesn't require people to use email > directly. Half of the pain in the process comes from coaxing email > clients that don't treat mail text as sacrosanct to leave it alone and > not mangle it. (Some of that would go away if we accepted attachments > with inline disposition, but not all of it. All of it would go away if > the submission process just involved "git push" to an appropriate > location.) But submission is less important than review. And for review it is usually better (except gigantic series) to have patch text for review with the review. And threading. And (meta)-versioning of series. And place for proof-of-concept / weather-balon patches... -- Jakub Narebski -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Wed, Aug 10, 2016 at 09:30:01AM +0200, Jakub Narębski wrote: > On 10 August 2016 at 02:55, Josh Triplettwrote: > > On Tue, Aug 09, 2016 at 06:28:00PM +, Eric Wong wrote: > >> Some of these problems I hope public-inbox (or something like > >> it) can fix and turn the tide towards email, again. > > > > This really seems like the dichotomy that drives people towards central > > services like GitHub or GitLab. We need an alternative that doesn't > > involve email, or at the very least, doesn't require people to use email > > directly. Half of the pain in the process comes from coaxing email > > clients that don't treat mail text as sacrosanct to leave it alone and > > not mangle it. (Some of that would go away if we accepted attachments > > with inline disposition, but not all of it. All of it would go away if > > the submission process just involved "git push" to an appropriate > > location.) > > But submission is less important than review. And for review it is > usually better (except gigantic series) to have patch text for review > with the review. Agreed. However, submission typically requires more work than review, because the patch text must remain applicable. For review, as long as the email client you use to respond doesn't do something horrible like *re-wrap* the quoted patch text, the result will work as a review. Ideally, I'd love to see 1) a review UI that stores line-by-line reviews into a common format and can translate those to email, and 2) a mechanism to translate reviews written by email and quoting into the review format and store them with the repository. > And (meta)-versioning of series. I've got a documented format for that. :) > And place for proof-of-concept / weather-balon patches... Same place as all other patches, just with an "RFC" tag on them. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Josh Triplettwrote: > On Tue, Aug 09, 2016 at 06:28:00PM +, Eric Wong wrote: > > Some of these problems I hope public-inbox (or something like > > it) can fix and turn the tide towards email, again. > > This really seems like the dichotomy that drives people towards central > services like GitHub or GitLab. We need an alternative that doesn't > involve email, or at the very least, doesn't require people to use email > directly. Half of the pain in the process comes from coaxing email > clients that don't treat mail text as sacrosanct to leave it alone and > not mangle it. (Some of that would go away if we accepted attachments > with inline disposition, but not all of it. All of it would go away if > the submission process just involved "git push" to an appropriate > location.) I don't mind patches as attachments and did some work a few months ago to ensure they're individually downloadable in the public-inbox WWW interface (along with full mboxrd messages)[1]. Fwiw, attachments are preferred in perl5-porters, and it might be acceptable on LKML, even. Not my call, though. Having a push/pull-only workflow would still require some sort of messaging system to notify others. Ideally that message would have the output of "git request-pull" to ensure people are on the same page; but I'd prefer patches (either attachments or inline) continue to be sent anyways in case the server is down or the reader is offline or on a machine without git. [1] see Brian's (who is new, here) initial email for diff-highlight: https://public-inbox.org/git/20160728162712.ga29...@tci.corp.yp.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Tue, Aug 09, 2016 at 06:28:00PM +, Eric Wong wrote: > Some of these problems I hope public-inbox (or something like > it) can fix and turn the tide towards email, again. This really seems like the dichotomy that drives people towards central services like GitHub or GitLab. We need an alternative that doesn't involve email, or at the very least, doesn't require people to use email directly. Half of the pain in the process comes from coaxing email clients that don't treat mail text as sacrosanct to leave it alone and not mangle it. (Some of that would go away if we accepted attachments with inline disposition, but not all of it. All of it would go away if the submission process just involved "git push" to an appropriate location.) - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Tue, Aug 09, 2016 at 06:57:05AM -0400, Jeff King wrote: > On Tue, Aug 09, 2016 at 10:11:30AM +0200, Michael J Gruber wrote: > > > Maybe two more points of input for the discussion: > > > > off-line capabilities > > = > > > > The current workflow here seems to work best when you are subscribed to > > the git-ml and have your own (local, maybe selective) copy of git-ml in > > your (text-based) MUA (mutt, (al)pine, emacs, ...) that can jump right > > into git-am and such directly. I'm not sure how important the "off-line" > > aspect of that is for some of us, and how that could be replicated on > > GitHub - while PRs and such may be Git based behind the scenes there > > seems to be no way to clone that info and work from a local clone. > > (Don't know if GitLab is more open.) > > You can pull it all out via GitHub's HTTP API, but the question is what > format you would use to store it locally (and which tools you would then > use to play with it). > > I haven't really tried this lately, though, so I don't know if there is > information that the API would be missing. > > I do have a dream of writing a tool that sucks in GitHub PRs to a fake > email thread, lets me make my responses inline in an editor, and then > pushes it back up as PR comments (finding the right positions based on > the quoted context). You might try https://github.com/joeyh/github-backup -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Jeff Kingwrites: > On Tue, Aug 09, 2016 at 10:34:11AM -0700, Junio C Hamano wrote: > >> It may be a good UI that is optimized for drive-by contributors. It >> is just that it is not very well suited (compared to mailing list >> discussions) to conduct high-volume exchange of ideas and changes >> efficiently. > > I think that's something to ponder; can we have a workflow where > drive-by contributors can use something that has a lower learning/setup > curve, but long-term contributors might opt for something more powerful? > > I think SubmitGit is a step in that direction. Yes, agreed 100% with that. The author of the tool must be praised by getting added to the Cc: line in this discussion ;-) > It does still require > switching to the mailing list for subsequent conversation, though. It > would be interesting to see something like SubmitGit that puts its own > email in the "From", and that processes email replies into PR comments, > and then subsequent PR comments into emails (i.e., part of my "dream tool" > from earlier). It's not clear to me whether the result would just end up > being irritating for both sides to use (because it doesn't _quite_ > conform to the norms of each format). But it would be fun to find out. Perhaps. I do not know if I like that second and subsequent steps for SubmitGit, but its first step as currently deployed I am very happy with. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Tue, Aug 09, 2016 at 11:50:51AM -0700, Stefan Beller wrote: > > Could you share your mutt set up pleaaase? I've been wanting this for > > a long time, but never used mutt long enough to bother with a proper > > setup like this (I blame gmail). > > > That is my complaint^H^H^H^H position, too. > I always wanted to switch to a more powerful > setup than git-send-email for sending /gmail for reading, > but I could not convince myself the steep learning/setup curve > is worth it eventually as it is "not broken enough" to do the change > right now. I think I may have shared it before, but here is the script I use to send emails. It dumps you in mutt, and then I have: macro index,pager b ":set edit_headers=yes:set edit_headers=no" to send the message ("b" is for "bounce", which I think may be another Pine-ism). -- >8 -- #!/bin/sh upstream_branch() { current=`git symbolic-ref HEAD` upstream=`git for-each-ref --format='%(upstream)' "$current"` if test -n "$upstream"; then echo $upstream else echo origin fi } get_reply_headers() { perl -ne ' if (defined $opt) { if (/^\s+(.*)/) { $val .= " $1"; next; } print "--$opt=", quotemeta($val), " "; $opt = $val = undef; } if (/^(cc|to):\s*(.*)/i) { $opt = lc($1); $val = $2; } elsif (/^message-id:\s*(.*)/i) { $opt = "in-reply-to"; $val = $1; } elsif (/^subject:\s*\[PATCH v(\d+)/i) { print "-v$1 "; } elsif (/^$/) { last; } ' } has_nonoption= for i in "$@"; do case "$i" in -[0-9]) has_nonoption=yes ;; -*) ;; *) has_nonoption=yes esac done git rev-parse || exit 1 : ${REPLY:=$HOME/patch} test -e "$REPLY" && eval "set -- `get_reply_headers <\"$REPLY\"` \"\$@\"" test "$has_nonoption" = "yes" || set -- "$@" `upstream_branch` git format-patch -s --stdout --from "$@" >.mbox if test -t 1; then mutt -e 'set sort=mailbox-order' -f .mbox else perl -lne ' if (/^Subject: (.*)/) { $subject = $1; } elsif ($subject && /^\s+(.*)/) { $subject .= " $1"; } elsif ($subject) { print $subject; $subject = undef; } ' .mbox | sed -e 's/\[PATCH /[/' \ -e 's/]/]:/' \ -e 's/^/ /' fi rm -f .mbox -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Tue, Aug 09, 2016 at 08:43:59PM +0200, Duy Nguyen wrote: > On Tue, Aug 9, 2016 at 1:37 PM, Jeff Kingwrote: > >That's (relatively) easy for me to script via mutt (grab > >these patches, apply them). > > Could you share your mutt set up pleaaase? I've been wanting this for > a long time, but never used mutt long enough to bother with a proper > setup like this (I blame gmail). It's actually pretty simple. The relevant config from my .muttrc is: macro pager,index D 'rm -f $HOME/patch' macro pager,index A '~/patch' I use "~/patch" as a rendezvous point, and then "git am ~/patch" from my other terminal. That avoids mutt having to know which repo to apply to, and keeps the "am" process in its own terminal (which is handy if it runs into conflicts, for example). So generally I would "D" to clear out the contents of ~/patch, and then "A" whichever patches I want to apply. I often use mutt's aggregate selection for that. My bindings are: bind index \; tag-pattern bind index a tag-prefix which I think come from pine (which I used for many years before switching to mutt probably 15 years ago). I don't recall the default keybindings. Anyway, you can either tag using a pattern (with ";"), or tag mails individually (using "t", the default), and then "a-A" to apply the "A" to all of them (if you are in the habit of tagging all of them and then doing "A" in one swoop, you could also get rid of the separate "D" command and just make "A" imply it). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Tue, Aug 9, 2016 at 11:43 AM, Duy Nguyenwrote: > On Tue, Aug 9, 2016 at 1:37 PM, Jeff King wrote: >>That's (relatively) easy for me to script via mutt (grab >>these patches, apply them). > > Could you share your mutt set up pleaaase? I've been wanting this for > a long time, but never used mutt long enough to bother with a proper > setup like this (I blame gmail). That is my complaint^H^H^H^H position, too. I always wanted to switch to a more powerful setup than git-send-email for sending /gmail for reading, but I could not convince myself the steep learning/setup curve is worth it eventually as it is "not broken enough" to do the change right now. My experiments with mutts, have left these lines in my ~/.muttrc > # use shift + A to apply a patch in the working dir > # macro index A ":unset pipe_decode\n|git am -3\n:set pipe_decode\n" > # macro pager A ":unset pipe_decode\n|git am -3\n:set pipe_decode\n" > > macro index A ":set folder='.'\n:copy-message\n" (IIRC they were broken for many patches, but I got applying one patch to work. Which sucks for long email series.) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Tue, Aug 9, 2016 at 1:37 PM, Jeff Kingwrote: >That's (relatively) easy for me to script via mutt (grab >these patches, apply them). Could you share your mutt set up pleaaase? I've been wanting this for a long time, but never used mutt long enough to bother with a proper setup like this (I blame gmail). -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Tue, Aug 9, 2016 at 8:36 PM, Duy Nguyenwrote: > On Tue, Aug 9, 2016 at 1:20 AM, Michael Haggerty wrote: >> Could you elaborate why you would expect quality and/or quantity of >> reviews to suffer? I'm really curious, and I'd be happy to pass your >> feedback along to my colleagues. > > Since I have been using github at work for a couple months, I do have > a few complaints if it will ever become the way of reviewing things in > git. Some of these may be covered by other people already (I haven't > read all new mails in this thread yet) Another super nit thing: use monospace font for commit messages, or at least have an option for that. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Tue, Aug 9, 2016 at 1:20 AM, Michael Haggertywrote: > Could you elaborate why you would expect quality and/or quantity of > reviews to suffer? I'm really curious, and I'd be happy to pass your > feedback along to my colleagues. Since I have been using github at work for a couple months, I do have a few complaints if it will ever become the way of reviewing things in git. Some of these may be covered by other people already (I haven't read all new mails in this thread yet) - Github PRs seem to encourage seeing changes as a whole, not a separate commits. Or at least it's not so convenient to view separate commits (e.g. not easy to go to the next commit) - The ability to show all outdated comments, in case I want to search through them. - I have a feeling that commits in PRs are sorted by authordate, not in topological order. The order of commits being committed is important sometimes. - Not showing leading spaces mixing with TABs, or trailing spaces - I would love to have all patches numbered so can refer to them as 1/7, 2/5... instead of just short sha1 (and I think you have the ability to refer to "1/7 of iteration 2", see next bullet point) -I guess you can manage multiple iterations of a topic with one iteration per PR, then linking them together. It would be nicer to somehow bake the iteration concept directly to a PR so we can switch between them, or do interdiff. I know, this is more of a improvement request than complaint because ML is not really better. - Offline support would be very nice. I'm only most of the time, but sometimes I do work on git stuff offline. - We lose the integration with ML, I think. Sometimes the user reports a bug here, then we reply back with a patch. With github, I guess we reply back with a PR number, then further discussion may go there, some discussion may still be on ML. > * It is easy to summon somebody else into the review conversation by > @-mentioning them. That person immediately can see the whole history of > the PR. (This is an improvement on the status quo, where a new entrant > to a conversation might have to dig through the email trash or an email > archive to see emails that they overlooked before they were added to the > CC list.) On the other hand, we can't just CC anybody anymore because we don't know if they have a github account (or the account name for that matter). Or does github allow @-ing email addresses too? We record people's email address, not github account names. > * It is possible to search old issues and PRs for additional context. > (Granted, the history of the project from its ML era would have to be > searched separately.) To me searching in email is still better. Maybe I haven't fully explored github search capabilities -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Michael Haggertywrote: > On 08/04/2016 05:58 PM, Johannes Schindelin wrote: > > [...] > > Even requiring every contributor to register with GitHub would be too much > > of a limitation, I would wager. > > [...] > * Discussion of pull requests can be done either > * via the website (super easy for beginners but powerful for > experienced users), > * by setting up email notifications for your account and replying to > those emails, or > * via an API. > Such discussion is all in markdown, which supports light formatting, > hyperlinks, and @-mentions. > Disclaimer: I work for GitHub, but in this email I'm speaking for myself. > > Michael > > [1] I concede that people who refuse on ideological grounds to use > proprietary software will find this step insurmountable. Perhaps we > could come up with a workaround for such people. I'm one of those ideological people and I don't see an acceptable workaround. GitHub already has misfeatures designed to lock people in into centralized messaging: * pull request feature doesn't work for self-hosted repos (this disincentivizes people from running and improving git-daemon/git-http-backend/etc...) * "noreply" email addresses * @-mentions you wrote about * custom email notifications This is a problem with Gitlab, Redmine, etc, too: they cannot interoperate with each other. At least for now, large proprietary mail providers like Gmail still interoperate with whatever Free Software SMTP software I run. I dread the day when that is no longer true. Some of these problems I hope public-inbox (or something like it) can fix and turn the tide towards email, again. In contrast, public-inbox is designed to push decentralization: * "reply" links are instructions for "git send-email" which encourage reply-to-all (this applies to what Jeff said about vger going down, I noticed it, too) * anybody can clone the code + repo, replicate the instances, and tweak it to their needs. * public-inbox.org/git/$MESSAGE_ID/t.atom allows subscriptions to Atom feeds without any registration or user-tracking * Message-IDs are exposed for proper threading and interop * low-bandwidth, Tor-friendly design to encourage deployments even behind NATs and firewalls. Anyways, my optimistic side might interpret your advocacy as GitHub already feeling threatened by public-inbox. I certainly wouldn't expect it at this stage, but I certainly hope it will be the case one day :) Disclaimer: I've always been willing to risk a lifetime of unemployment for ideology. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Tue, Aug 09, 2016 at 10:34:11AM -0700, Junio C Hamano wrote: > >The threading in GitHub comments and pull requests is also not great. > >Each PR or issue is its own thread, but it's totally flat inside. > >There are no sub-threads to organize discussion, and it's sometimes > >hard to see what people are replying to. > > It may be a good UI that is optimized for drive-by contributors. It > is just that it is not very well suited (compared to mailing list > discussions) to conduct high-volume exchange of ideas and changes > efficiently. I think that's something to ponder; can we have a workflow where drive-by contributors can use something that has a lower learning/setup curve, but long-term contributors might opt for something more powerful? I think SubmitGit is a step in that direction. It does still require switching to the mailing list for subsequent conversation, though. It would be interesting to see something like SubmitGit that puts its own email in the "From", and that processes email replies into PR comments, and then subsequent PR comments into emails (i.e., part of my "dream tool" from earlier). It's not clear to me whether the result would just end up being irritating for both sides to use (because it doesn't _quite_ conform to the norms of each format). But it would be fun to find out. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Jeff Kingwrites all what I wanted to say, and a lot more, so I don't have to say much ;-) > - I really like the flow of having conversations next to patches. I can >look at the index of the mailing list folder and see what people are >talking about, how big the threads are, etc, at a glance. Moving >between messages and threads involve single keystrokes. > >Similarly, having local storage is _fast_. I think GitHub is fine for >a web app. But when I'm reading a high-volume mailing list, I really >want to flip around quickly. If there's even 500ms to get to the next >message or thread, it feels clunky and slow to me. Obviously I spend >more than 500ms _reading_ most messages (though for some I see the >first paragraph and immediately jump away). It's just the latency >when I've decided I'm done with one thing and want to move to the >next. Viewing threads in a threaded mail client to help prioritizing various topics being discussed is what I value the most and I am not sure how I can be as efficient with the pull-request page. >The threading in GitHub comments and pull requests is also not great. >Each PR or issue is its own thread, but it's totally flat inside. >There are no sub-threads to organize discussion, and it's sometimes >hard to see what people are replying to. It may be a good UI that is optimized for drive-by contributors. It is just that it is not very well suited (compared to mailing list discussions) to conduct high-volume exchange of ideas and changes efficiently. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Johannes Schindelinwrites: > On Mon, 8 Aug 2016, Junio C Hamano wrote: > >> Unless a patch is about an area you are super familiar with so that you >> know what is beyond the context of the patch to be able to judge if the >> change is good in the context of the file being touched, it is always >> hard to review from inside a mail reader. >> >> Running "git am" is a good first step to review such a patch, as that >> lets you view the resulting code with the full power of Git. As you >> gain experience on the codebase, you'll be able to spot more problems >> while in your mail reader. > > I am glad that you agree that the requirement to manually transform the > patches back into Git (where they had been originally to begin with) is > cumbersome. This is the first time that I see you admit it ;-) I was about to apologize for writing a statement that can be misread, but I do not think what I wrote can be misinterpreted, even if a reader deliberately tries to twist the words s/he reads, to lead to such a conclusion, so I won't. I merely said that reviewing a change in an unfamiliar area is harder (not "cumbersome", but "needs understanding first") with a patch, and it is easier to see changes in context by applying (which is an easy, not "cumbersome", process). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Michael, On Tue, 9 Aug 2016, Michael Haggerty wrote: > On 08/04/2016 05:58 PM, Johannes Schindelin wrote: > > [...] > > Even requiring every contributor to register with GitHub would be too > > much of a limitation, I would wager. > > [...] > > Is it *really* so insane to consider moving collaboration on the Git > project to GitHub or some other similar platform? Speaking for myself, I do prefer GitHub's UI to mail, by a lot. Not only because it is more focused on the code, but because it integrates so nicely with Git, which email distinctly does not. So I personally would not have the least bit of a problem to switch to GitHub (that's indeed what Git for Windows did, getting substantially more contributions than we would otherwise have). And of course I use the email notifications quite a bit. They are really convenient: I get my updates via my mail program, still, and the discussion I want to participate in is just one click away. The reason why I stated that GitHub is out of the question is that I expected resistance against it. But you are right: I should not have ruled it out so categorically, it is not at all my call to make. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Thunderbird woes, was: Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Jakub Narębski venit, vidit, dixit 09.08.2016 10:24: > On 9 August 2016 at 10:11, Michael J Gruberwrote: > >> My own setup >> >> >> My usual MUA is Thunderbird because of its integration with calendars >> and address books. I usually read and post to mailing lists via >> nntp/gmane, that works best for me for several reasons (e.g. archive >> available when I need it). >> >> For git-ml, I had to learn early on to answer by e-mail to git-ml rather >> than by nntp-reply because proper nntp-replies somehow didn't meet the >> expectations of the e-mail users (double copies because of the cc-policy >> or such, I don't remember). > > I use either KNode or Thunderbird with NNTP/Gmane, and I don't have > any problems when doing "Reply All" even if I forget to remove nntp-reply. > You should do reply-all anyway, because not everyone is subscribed, > and not everyone uses nntp-ml. There used to be a problem (many years ago), so let's see... > >> I use git sendemail even for small throw-in patches because the git-ml >> does not allow attachments but wants patches (files) as in-line text, >> and Thunderbird possibly reformats text (because it's text, you know). > > For some strange reason Thunderbird used as NNTP reader does not > screw up with whitespace, while Thunderbird used as email client does. > Al least it did last time I forgot to create new email in its NNTP reader. You mean, "format fl[ao]wed" is not used when posting via NNTP? That still wouldn't help with reply all. > Note that git-format-patch has Thunderbird subsection in the > "MUA specific hints" section. I know, but that changes config (esp. wrap+flowed) for all e-mails. I would be nice if using an external editor would turn off TB's mangling. >> When I want to try out a proposed patch I have to "save message" and run >> git-am because patches don't come as file attachments on the git-ml >> (can't use "save/open attachment"+ git-apply) nor a PR (can't use git >> fetch nor view in browser). > > Inline are preferred over attachment because it is easier to review > and comment on online patches. Anyway, it is the same amount of > steps, and git-am preserves commit message. I know why we do that, but attachments are there for a reason (stable transport), and sending attached patches would be much easier for many MUAs. It just shows that for nowadays usage of e-mail (not withstanding Junio's historic remarks), using in-line text as a stable transport is really "very special", to put it mildly. >>If it's a series, I have to do that >> for each >> invididual patch, which usually means I simply don't (or rely on Junio >> doing it and fetch his xy/topic branch). > > I think you can save-all on the whole thread... I'll definitely try, thanks. Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Junio, On Mon, 8 Aug 2016, Junio C Hamano wrote: > Johannes Schindelinwrites: > > > I think you both got it wrong. The original citizens were the mail > > clients that allowed you to communicate with other human beings. > > ... It is our usage to transport machine-readable content (and not > > as an attachment!) that is the intruder. > > It is not "its is our usage". > > You are too young to remember or too old to remember the history, or > you are knowingly distorting it. The original users of "patch" and > "diff" expected that e-mail to be a medium to safely exchange > changes to programs among themselves. If you are saying that transporting patches via email was the original purpose of email, then it is not exactly I who is misremembering history. But that is not what you meant, I believe. You probably wanted to point out that the Git developers are not the first ones to abuse the medium known as email that way. And you are correct, of course. And I never claimed anything else. I just said that the problem is our usage of emails as a means to transport byte-exact content intended primarily to be consumed by a program instead of a human. It does not matter whether others did that before us. It is the problem we face right now, that is the important part of my message. And even if it seems as if you are eagerly defending this system, I do not believe even a microsecond that you think it is a good system. I believe that you, too, would welcome a better review/contribution system that is easier to use, more welcoming to new users, less error-prone and less time-wasting than the current, email-based one, just like you jumped on Git as a better SCM when it came around, from whatever inadequate system you came from. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Junio, On Mon, 8 Aug 2016, Junio C Hamano wrote: > Lars Schneiderwrites: > > > 4.) Reviewing patches is super hard for me because my email client > > does not support patch color highlighting and I can't easily expand > > context or look at the history of code touched by the patch (e.g via > > git blame). I tried to setup Alpine but I wasn't happy with the > > interface either. I like patches with a GitHub URL for review but then > > I need to find the right line in the original email to write a > > comment. > > Unless a patch is about an area you are super familiar with so that you > know what is beyond the context of the patch to be able to judge if the > change is good in the context of the file being touched, it is always > hard to review from inside a mail reader. > > Running "git am" is a good first step to review such a patch, as that > lets you view the resulting code with the full power of Git. As you > gain experience on the codebase, you'll be able to spot more problems > while in your mail reader. I am glad that you agree that the requirement to manually transform the patches back into Git (where they had been originally to begin with) is cumbersome. This is the first time that I see you admit it ;-) Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Tue, Aug 09, 2016 at 01:20:05AM +0200, Michael Haggerty wrote: > > but I > > do not think it is sane to expect that the same quality and quantity > > of reviews as we do here can be maintained with that thing. > > Could you elaborate why you would expect quality and/or quantity of > reviews to suffer? I'm really curious, and I'd be happy to pass your > feedback along to my colleagues. Having done a lot of review here on the mailing list, as well as in GitHub PRs, I vastly prefer the mailing list. Here's a random list of things that I think I would miss: - I really like the flow of having the commit message and diff dumped in my editor. I'm very efficient at slicing and dicing text, omitting uninteresting quoted bits, etc. Web text boxes feel like a straitjacket. I do have a browser plugin that opens them in vim. That helps, but it breaks the flow (I make a comment, save the file, click "comment", then read to the next place, click "+", then start a new vim instance for that comment). Besides the tedium of clicking around, it loses the "unit" size of a single email, where I may make many comments, go back and revise earlier comments after reading more of the patch, etc. - I really like the flow of having conversations next to patches. I can look at the index of the mailing list folder and see what people are talking about, how big the threads are, etc, at a glance. Moving between messages and threads involve single keystrokes. Similarly, having local storage is _fast_. I think GitHub is fine for a web app. But when I'm reading a high-volume mailing list, I really want to flip around quickly. If there's even 500ms to get to the next message or thread, it feels clunky and slow to me. Obviously I spend more than 500ms _reading_ most messages (though for some I see the first paragraph and immediately jump away). It's just the latency when I've decided I'm done with one thing and want to move to the next. - For that matter, GitHub doesn't really have a good tool for random conversations. There are issues, which you can vaguely use like a thread, but it doesn't quite feel the same. I think part of it is that I can view the mailing list both as a series of threads _and_ as a stream of messages. So sometimes I mark a thread as "read", and then see the next day that there are a ton of new messages on it. Maybe those are uninteresting (and it's a single keystroke to mark the thread again), but maybe that's a hint that there's interesting discussion going on. The threading in GitHub comments and pull requests is also not great. Each PR or issue is its own thread, but it's totally flat inside. There are no sub-threads to organize discussion, and it's sometimes hard to see what people are replying to. - When I move between a discussion and patches on the list and my local git checkout, it's important to do so with minimal fuss. Which means I want to use _context_ in my workflow. If I'm reading a thread, I want there to be a keystroke for "jump to this thread in my checkout". That's (relatively) easy for me to script via mutt (grab these patches, apply them). It's a bit harder in the browser (the best I've got is to copy-paste the URL to a script that pulls out the PR number, then fetches and checks it out). - A sort-of feature: the mailing list is actually fairly decentralized, because of the "reply-to-all" convention. I don't know if anybody else noticed, but vger seemed to be down Friday evening and Saturday morning (at least my messages to the list got 400 SMTP codes, and no new messages were delivered to me). But I still had some conversations going with people, because our messages were mailed directly (and the list eventually caught up). Now that probably doesn't matter for GitHub, which seems to have fairly reasonable uptime. It would matter if we picked a centralized tool that didn't. There are probably more, but I've run out of ranting steam for now. :) > Here are some factors that I think will *improve* reviews: I was going to respond point-by-point to a few of these, but I think I covered most of it above. In short, I agree with many of the benefits you list. In most cases, I've already reaped those benefits for my own workflow (e.g., my "git am" workflow is pretty efficient now). But not everybody has done so, and it's a lot to ask of casual contributors. > Given that I work for GitHub, I'm uncomfortable doing any more advocacy > here. If people have concrete questions, I'd be happy to answer them, on > the list or in private. Hopefully I provided some counterpoint. ;) -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Tue, Aug 09, 2016 at 10:11:30AM +0200, Michael J Gruber wrote: > Maybe two more points of input for the discussion: > > off-line capabilities > = > > The current workflow here seems to work best when you are subscribed to > the git-ml and have your own (local, maybe selective) copy of git-ml in > your (text-based) MUA (mutt, (al)pine, emacs, ...) that can jump right > into git-am and such directly. I'm not sure how important the "off-line" > aspect of that is for some of us, and how that could be replicated on > GitHub - while PRs and such may be Git based behind the scenes there > seems to be no way to clone that info and work from a local clone. > (Don't know if GitLab is more open.) You can pull it all out via GitHub's HTTP API, but the question is what format you would use to store it locally (and which tools you would then use to play with it). I haven't really tried this lately, though, so I don't know if there is information that the API would be missing. I do have a dream of writing a tool that sucks in GitHub PRs to a fake email thread, lets me make my responses inline in an editor, and then pushes it back up as PR comments (finding the right positions based on the quoted context). > For git-ml, I had to learn early on to answer by e-mail to git-ml rather > than by nntp-reply because proper nntp-replies somehow didn't meet the > expectations of the e-mail users (double copies because of the cc-policy > or such, I don't remember). At least some people's workflows seem to send two copies to the list. For instance, Jakub's <2bfd9cf5-a9fa-7650-21e9-9ceb9cc34...@gmail.com> got delivered to me via the list twice. Once directly from gmail with: To: Oleg Taranenko, Junio C Hamano Cc: git@vger.kernel.org and once via gmane with: To: git@vger.kernel.org Cc: git@vger.kernel.org It's like this with all of his messages (sorry I can't point to the duplicates in an archive; they have the same message-id, so public-inbox treats them as a single unit). Replying to the second one breaks the usual "cc-everybody" rule. Sending duplicates means everybody sees it twice (3 times if they're on the cc list!), and the second copy still has the bogus headers (so people replying need to pick the right one). > I use git sendemail even for small throw-in patches because the git-ml > does not allow attachments but wants patches (files) as in-line text, > and Thunderbird possibly reformats text (because it's text, you know). I wonder if this is something we could change. I do not personally have any problem with attached patches. "git am" knows how to apply them, and mutt is smart enough to show text/* by default, and to include it in quoted text on reply. So the output of "git format-patch --attach" works fine for me. But it may not be as nice in other MUAs, and we have to care about all of the other reviewers. > When I want to try out a proposed patch I have to "save message" and run > git-am because patches don't come as file attachments on the git-ml > (can't use "save/open attachment"+ git-apply) nor a PR (can't use git > fetch nor view in browser). If it's a series, I have to do that for each > invididual patch, which usually means I simply don't (or rely on Junio > doing it and fetch his xy/topic branch). So you would like the opposite of my dream tool, I think: something that takes mailing list conversations and turns them into PRs. (My real dream is actually to have a bidirectional version of the tool, so that everybody can use whatever interface they like, and nobody has to care about somebody else's preferences). > And more often than not, patches from series do not appear in sequence, > not threaded on top of the cover letter (in the gmane nntp version of > git-ml), and it usually happens for the same people again and again, > which tells me it's a git sendemail config issue and not gmane. Just a guess, but I suspect this is caused by people who use "rebase -i" to rearrange patches. When format-patch writes out the patches, it uses the author date as the "Date" field, which means it may be out of order. I think send-email will always write out a new, monotonically increasing date. But I suspect other workflows (e.g., imap-send and then mailing from a MUA) blindly re-use that date. > Suggestion > == > > Maybe the current gmane woes are a good reason to try out something > different for a month or two, with copies to the git-ml, and with the > default being to revert back to git-ml after that and discuss what we've > learned. As a result we may improve our workflow here, get GitHub to > improve, and maybe switch or not. Either way we could profit from that. I think public-inbox is a nice step forward on the reading side (it's a lot easier to get raw patches out of it, for example). But it doesn't help much with sending (and sending is a tricky subject; anytime you promise to send mail on behalf of
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Tue, Aug 09, 2016 at 10:17:22AM +0100, Richard Ipsum wrote: > > In the very unlikely event that github is shut down, how do we get all > > review comments out of it, assuming that we will use pull requests for > > review? > > For what it's worth this is exactly why I think it would be worthwhile for git > to establish a common review format, services like Github/Gitlab could then > start storing reviews and comments in the git repo rather than in a separate > sql database. I doubt that the "rather than" part will ever happen. Git does not make a very good database, and certainly not when you want to do things that cut across repositories (like, say, efficiently get all review comments made by one user). It would be nice to have a common interchange format, though. In theory that could feed into (and out of) a more efficient representation on the backend of the site. It doesn't _have_ to be git-based, but it would be nice if it was. Somebody asked elsewhere "what happens if GitHub goes away?". And the answer is that you can already get all of that data out in a programmatic way, via the API. But since there's no common interchange format, you'd be stuck writing a conversion to whatever format your new destination uses. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On 08/09/2016 06:22 AM, Duy Nguyen wrote: > On Tue, Aug 9, 2016 at 12:20 AM, Michael Haggerty> wrote: >> On 08/04/2016 05:58 PM, Johannes Schindelin wrote: >>> [...] >>> Even requiring every contributor to register with GitHub would be too much >>> of a limitation, I would wager. >>> [...] >> >> Is it *really* so insane to consider moving collaboration on the Git >> project to GitHub or some other similar platform? > > In the very unlikely event that github is shut down, how do we get all > review comments out of it, assuming that we will use pull requests for > review? I don't have any experience with these tools, but a quick search turns up the following possibilities (among others): * github-backup (by Joey Hess): https://github.com/joeyh/github-backup * python-github-backup: https://github.com/josegonzalez/python-github-backup * BackHub (commercial service): https://backhub.co/ * Import GitHub project into GitLab: http://docs.gitlab.com/ce/workflow/importing/import_projects_from_github.html Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Tue, Aug 09, 2016 at 06:22:21AM +0200, Duy Nguyen wrote: > On Tue, Aug 9, 2016 at 12:20 AM, Michael Haggerty> wrote: > > On 08/04/2016 05:58 PM, Johannes Schindelin wrote: > >> [...] > >> Even requiring every contributor to register with GitHub would be too much > >> of a limitation, I would wager. > >> [...] > > > > Is it *really* so insane to consider moving collaboration on the Git > > project to GitHub or some other similar platform? > > In the very unlikely event that github is shut down, how do we get all > review comments out of it, assuming that we will use pull requests for > review? For what it's worth this is exactly why I think it would be worthwhile for git to establish a common review format, services like Github/Gitlab could then start storing reviews and comments in the git repo rather than in a separate sql database. Gerrit is already doing this with notedb, which literally gives you a git log of a review. Admittedly with Gerrit the change metadata sits in a separate git repo, still, this is much better than the current situation with Github and Gitlab in my opinion. I apologise once again if my comments here are somewhat unrelated, but I feel there is at least some overlap, since the existence of a common review format for git could potentially make Github/Gitlab a more attractive option, if Github/Gitlab chose to adopt such a format. Really I think that reviews shouldn't be stored on mailing lists, and they shouldn't be stored in sql databases, they should be stored in git. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thunderbird woes, was: Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On 9 August 2016 at 10:11, Michael J Gruberwrote: > My own setup > > > My usual MUA is Thunderbird because of its integration with calendars > and address books. I usually read and post to mailing lists via > nntp/gmane, that works best for me for several reasons (e.g. archive > available when I need it). > > For git-ml, I had to learn early on to answer by e-mail to git-ml rather > than by nntp-reply because proper nntp-replies somehow didn't meet the > expectations of the e-mail users (double copies because of the cc-policy > or such, I don't remember). I use either KNode or Thunderbird with NNTP/Gmane, and I don't have any problems when doing "Reply All" even if I forget to remove nntp-reply. You should do reply-all anyway, because not everyone is subscribed, and not everyone uses nntp-ml. > I use git sendemail even for small throw-in patches because the git-ml > does not allow attachments but wants patches (files) as in-line text, > and Thunderbird possibly reformats text (because it's text, you know). For some strange reason Thunderbird used as NNTP reader does not screw up with whitespace, while Thunderbird used as email client does. Al least it did last time I forgot to create new email in its NNTP reader. Note that git-format-patch has Thunderbird subsection in the "MUA specific hints" section. > When I want to try out a proposed patch I have to "save message" and run > git-am because patches don't come as file attachments on the git-ml > (can't use "save/open attachment"+ git-apply) nor a PR (can't use git > fetch nor view in browser). Inline are preferred over attachment because it is easier to review and comment on online patches. Anyway, it is the same amount of steps, and git-am preserves commit message. >If it's a series, I have to do that > for each > invididual patch, which usually means I simply don't (or rely on Junio > doing it and fetch his xy/topic branch). I think you can save-all on the whole thread... Best, -- Jakub Narębski -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Michael Haggerty venit, vidit, dixit 09.08.2016 01:20: > Given that I work for GitHub, I'm uncomfortable doing any more advocacy > here. If people have concrete questions, I'd be happy to answer them, on > the list or in private. You're doing a great job differentiating between your roles as a member of the Git devel community and as a GitHub employee, so please keep the discussion here. Maybe two more points of input for the discussion: off-line capabilities = The current workflow here seems to work best when you are subscribed to the git-ml and have your own (local, maybe selective) copy of git-ml in your (text-based) MUA (mutt, (al)pine, emacs, ...) that can jump right into git-am and such directly. I'm not sure how important the "off-line" aspect of that is for some of us, and how that could be replicated on GitHub - while PRs and such may be Git based behind the scenes there seems to be no way to clone that info and work from a local clone. (Don't know if GitLab is more open.) My own setup My usual MUA is Thunderbird because of its integration with calendars and address books. I usually read and post to mailing lists via nntp/gmane, that works best for me for several reasons (e.g. archive available when I need it). For git-ml, I had to learn early on to answer by e-mail to git-ml rather than by nntp-reply because proper nntp-replies somehow didn't meet the expectations of the e-mail users (double copies because of the cc-policy or such, I don't remember). I use git sendemail even for small throw-in patches because the git-ml does not allow attachments but wants patches (files) as in-line text, and Thunderbird possibly reformats text (because it's text, you know). When I want to try out a proposed patch I have to "save message" and run git-am because patches don't come as file attachments on the git-ml (can't use "save/open attachment"+ git-apply) nor a PR (can't use git fetch nor view in browser). If it's a series, I have to do that for each invididual patch, which usually means I simply don't (or rely on Junio doing it and fetch his xy/topic branch). And more often than not, patches from series do not appear in sequence, not threaded on top of the cover letter (in the gmane nntp version of git-ml), and it usually happens for the same people again and again, which tells me it's a git sendemail config issue and not gmane. So really, everytime I interact with the git-ml I think about switching to mutt or such just for git-ml, even though over time I have gotten used to the number of hoops that I have to jump through if I want to interact with git-ml. Suggestion == Maybe the current gmane woes are a good reason to try out something different for a month or two, with copies to the git-ml, and with the default being to revert back to git-ml after that and discuss what we've learned. As a result we may improve our workflow here, get GitHub to improve, and maybe switch or not. Either way we could profit from that. Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Tue, Aug 9, 2016 at 12:20 AM, Michael Haggertywrote: > On 08/04/2016 05:58 PM, Johannes Schindelin wrote: >> [...] >> Even requiring every contributor to register with GitHub would be too much >> of a limitation, I would wager. >> [...] > > Is it *really* so insane to consider moving collaboration on the Git > project to GitHub or some other similar platform? In the very unlikely event that github is shut down, how do we get all review comments out of it, assuming that we will use pull requests for review? -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On 08/09/2016 12:36 AM, Junio C Hamano wrote: > Michael Haggertywrites: > >> Is it *really* so insane to consider moving collaboration on the Git >> project to GitHub or some other similar platform? > > I only know how "pull requests" and "issues" discussion in GitHub > Web interface _currently_ looks like, so if you have even more > wonderful thing in the works, I _might_ be swayed otherwise, If I did I couldn't say anyway, so let's assume that current GitHub is what's on the table [1]. There are a couple of recent code-review improvements that you might have missed: * You can now get email updates about your own activity [2]. (Previously you would only get emails about the activity of other people, which would leave holes in the email record of the conversation.) * There is also now better visibility of code review comments regarding lines that are no longer part of a PR [3]. > but I > do not think it is sane to expect that the same quality and quantity > of reviews as we do here can be maintained with that thing. Could you elaborate why you would expect quality and/or quantity of reviews to suffer? I'm really curious, and I'd be happy to pass your feedback along to my colleagues. Here are some factors that I think will *improve* reviews: * While you are reviewing patches, you can "zoom out" to see code beyond the usual diff context. Currently a reviewer who wants more context has to transition from reading the diff in email to applying the patch and viewing it in another tool. Then the reviewer has to go back to email to leave the comment. * If you want to compile/run/edit/profile the code, you just need to "git fetch" rather than messing around with "git am". For more involved suggestions, it is possible to propose a PR against the original PR. * It is easy to summon somebody else into the review conversation by @-mentioning them. That person immediately can see the whole history of the PR. (This is an improvement on the status quo, where a new entrant to a conversation might have to dig through the email trash or an email archive to see emails that they overlooked before they were added to the CC list.) * It is easy to subscribe/unsubscribe from particular discussions [4]. This makes it easier to follow the discussions you are interested in without getting swamped with emails about other discussions. You can unsubscribe from a discussion permanently, or in such a way that a new @-mention brings you back in. * It is easy to mention other PRs/commits/issues in a discussion, and those mentions become clickable links (no jumping back and forth between email client and web browser). Of course you can also link to arbitrary URLs (e.g., mailing list archives). * It is possible to search old issues and PRs for additional context. (Granted, the history of the project from its ML era would have to be searched separately.) Given that I work for GitHub, I'm uncomfortable doing any more advocacy here. If people have concrete questions, I'd be happy to answer them, on the list or in private. Michael [1] In general, GitHub *does* get better over time, and we would benefit from any future improvements. [2] https://github.com/blog/2203-email-updates-about-your-own-activity [3] https://github.com/blog/2123-more-code-review-tools [4] https://github.com/blog/2183-improvements-to-notification-emails -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Michael Haggertywrites: > Is it *really* so insane to consider moving collaboration on the Git > project to GitHub or some other similar platform? I only know how "pull requests" and "issues" discussion in GitHub Web interface _currently_ looks like, so if you have even more wonderful thing in the works, I _might_ be swayed otherwise, but I do not think it is sane to expect that the same quality and quantity of reviews as we do here can be maintained with that thing. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On 08/04/2016 05:58 PM, Johannes Schindelin wrote: > [...] > Even requiring every contributor to register with GitHub would be too much > of a limitation, I would wager. > [...] Is it *really* so insane to consider moving collaboration on the Git project to GitHub or some other similar platform? * Many, MANY of the most prominent open-source projects are already using GitHub. Many potential contributors already know how to use it and already have accounts. Casual observers (e.g., people who only want to clone the repo and/or read issues and PRs) don't even need an account. * Even if you don't already have a GitHub account, it is vastly easier to create one than to set up our current contribution workflow: figure out the correct SMTP settings for your email provider, configure git-send-email, test it and work out the kinks, figure out how to use git-am (and even then, actually using git-am is a tedious chore for people who don't use an email client that can run it automatically) [1]. We've seen how difficult our current workflow is by observing GSoC candidates attempting to send their first patch. What we haven't seen is the invisible GSoC candidates and other potential contributors who never even get as far as attempting to send a patch. * Interactions that involve code are done using Git commands directly, via exchanging bona fide Git commits. Which means that... * Commits have unambiguous SHA-1s, which we can use when discussing them, linking to them, merging them, etc. It will no longer be a matter of detective work to find out whether a discussion is about v1 or v3 of a patch series, let alone v3 with some cleanups that were silently added by Junio. * Discussion of pull requests can be done either * via the website (super easy for beginners but powerful for experienced users), * by setting up email notifications for your account and replying to those emails, or * via an API. Such discussion is all in markdown, which supports light formatting, hyperlinks, and @-mentions. * GitHub creates permalink URLs for all of the important artifacts: commits, pull requests, pull request comments, etc. These all can be referenced easily from any discussion. This web of cross-links accumulates over time and adds a lot of context to discussions. * GitHub keeps spam under control. I admit that if we move to GitHub we would be vulnerable if the company turns evil or goes bankrupt. But is that really a bigger risk than we accepted by relying on Gmane (a one-person hobbyist operation) for many of our historical permalinks, which are now broken? In any case, each of us has a mirror of the code, and there are utilities for backing up other GitHub metadata. *If* GitHub becomes evil, there will be a lot of other open-source projects in the same boat, so I am confident that the tooling for salvaging such information will quickly become excellent. Currently we force potential Git contributors to learn an email-based workflow that is almost unique to this project, rather than steering them towards a workflow (Git plus, potentially, GitHub) that they probably already know, or if not is worth learning because the knowledge will carry across to most other open-source projects, not to mention their professional careers. We would want to set down guidelines for how we use GitHub. For example, we might insist that each version of a patch series (v1, v2, etc.) be submitted as a fresh pull request with references to the previous version(s) (which also automatically creates forwards links from the previous versions to the new version). We might want to set up some robots to help with repetitive activities, like style review, pinging the right people, etc. Junio, I'm very sensitive to the need not to decrease your efficiency. But isn't there a chance that this could *increase* your efficiency? Is it worth an experiment? Is the Git project really such a unique snowflake that we need to use a workflow (and force it on our contributors) that is different than the workflows used by most other open-source projects? Disclaimer: I work for GitHub, but in this email I'm speaking for myself. Michael [1] I concede that people who refuse on ideological grounds to use proprietary software will find this step insurmountable. Perhaps we could come up with a workaround for such people. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Lars Schneiderwrites: > ... then I am always super > eager to send out a new roll just because I don't want any other reviewer > to waste time on obviously wrong patches. However, I have the impression > that frequent re-rolls are frowned upon. Correct. A good middle-ground is to just reply with "Yes, thanks for your suggestion, will fix in the next round", while receiving review comments. Good reviewers who value their time will not to waste their time by responding on a point that has already been pointed out and acknowledged. > 4.) Reviewing patches is super hard for me because my email client does not > support patch color highlighting and I can't easily expand context or > look at > the history of code touched by the patch (e.g via git blame). I tried to > setup > Alpine but I wasn't happy with the interface either. I like patches with > a GitHub > URL for review but then I need to find the right line in the original > email to > write a comment. Unless a patch is about an area you are super familiar with so that you know what is beyond the context of the patch to be able to judge if the change is good in the context of the file being touched, it is always hard to review from inside a mail reader. Running "git am" is a good first step to review such a patch, as that lets you view the resulting code with the full power of Git. As you gain experience on the codebase, you'll be able to spot more problems while in your mail reader. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Johannes Schindelinwrites: > I think you both got it wrong. The original citizens were the mail > clients that allowed you to communicate with other human beings. > ... It is our usage to transport machine-readable content (and not > as an attachment!) that is the intruder. It is not "its is our usage". You are too young to remember or too old to remember the history, or you are knowingly distorting it. The original users of "patch" and "diff" expected that e-mail to be a medium to safely exchange changes to programs among themselves. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
> On 06 Aug 2016, at 10:58, Johannes Schindelin> wrote: > > Hi Stefan, > > just quickly (i.e. addressing only one point, will try to address more at > a later date) because I want to be mostly offline this weekend: > > On Fri, 5 Aug 2016, Stefan Beller wrote: > >> On Fri, Aug 5, 2016 at 1:20 AM, Johannes Schindelin >> wrote: >>> >>> I also hate to break it to you that git-send-email is not going to be >>> part of any solution. >> >> It's written in perl, so it's not one of the core parts of Git that you >> mentioned above. I do use it though for my submission process. > > The problem is not Perl, but how fiddly it is to set up. And that you lose > all the niceties of an email client (e.g. when you want to add a comment > before the diff stat that is not intended to become a part of the commit > message). > > But I had an apostrophe last night. I might have been a bit overzealous to > claim that git-send-email won't be a part of the solution. It cannot be > a *user-visible* part of any solution, that still holds true. > > So here is the apostrophe: why not implement a bot that listens to the PRs > on GitHub, and accepts commands such as "@bot please send this > upstream" via comments on the PR. It would then send the patches to the > list, from its own email address, on behalf of the contributor. > > Lots of things to kink out, such as: does it need to be moderated? Record > what was submitted in its own git.git fork? Accept replies and attach them > to the correct PR? Maybe annotate those replies with the commits whose > patches were discussed? Maybe send out replies on the PR as emails? Maybe > try to auto-apply suggested patches? Cc: people who commented on earlier > iterations of the patch series? Maybe make interaction smarter using an AI > bot framework? > > If only I had lots of time on my hand, I'd start by prototyping a node.js > server and hooking it up via webhooks, then show it off so others can > tinker with it. > > This is not completely unlike submitGit, which was a good first attempt, > but I never used it because I needed it to do so much more than it already > did, *and* it complicated everything by requiring users to register with > an extra step to allow submitGit to send email on their behalf. It also > made contributing to it harder by choosing some non-standard web app > framework. Also, I really do not like having to go to a different website > just to send a GitHub PR to the list. > > Anyway, that was my brain fart for the day. Great discussion! I would like to share my perspective a someone who is a (relatively speaking) new Git contributor and who has never interacted on mailing lists before Git: 1.) "git format-patch" and "git send-email" work great for me. It took some time to learn how they work but now I have my own "submit.sh" based on those tools and posting a new series is a piece of cake. 2.) Initially it was hard for me to ensure that my patches don't break build or tests on Linux and OS X. Travis CI helps me a lot. I just wished we could get Windows support, too. 3.) I noticed that I get two types of reviews. The first kind points out things in my patch that are obviously wrong. The second kind are things that require a discussion. When I get feedback of the first kind, then I am always super eager to send out a new roll just because I don't want any other reviewer to waste time on obviously wrong patches. However, I have the impression that frequent re-rolls are frowned upon. If we would use Git for the patches instead of email, then I could add "squash" patches to indicate changes in the current roll that will be squashed in the next roll (I know I could send squash patches as email, too... but for me that gets confusing quickly). 4.) Reviewing patches is super hard for me because my email client does not support patch color highlighting and I can't easily expand context or look at the history of code touched by the patch (e.g via git blame). I tried to setup Alpine but I wasn't happy with the interface either. I like patches with a GitHub URL for review but then I need to find the right line in the original email to write a comment. Again, this is just my point of view as a "newbie" and I definitively don't expect the Git community to change their established workflows. Cheers, Lars-- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi, On Sat, 6 Aug 2016, Eric Wong wrote: > Junio C Hamanowrote: > > Somebody mentioned "configuring it is hard--why does the user have > > to know SMTP subtleties", and that may be a valid complaint against > > the primary part of send-email. The solution for that is not to > > discard it with bathwater, but make it just as easy as other MSAs, > > say, Thunderbird, to configure for an average user who can configure > > these other MUAs. > > Sadly, the average user does not use an MUA, SMTP or IMAP, anymore. > It's all webmail or apps using proprietary protocols. > Embrace, extend, extinguish :< I think you both got it wrong. The original citizens were the mail clients that allowed you to communicate with other human beings. Webmail is just a new generation of the same commodity. It is our usage to transport machine-readable content (and not as an attachment!) that is the intruder. It's not making things better if we require users to use a second mail client for sending out patches, and, oh, it does nothing to help with reintegrating patches back into Git, were they had been before taking that perilous and lossy journey through that medium called email. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Stefan, just quickly (i.e. addressing only one point, will try to address more at a later date) because I want to be mostly offline this weekend: On Fri, 5 Aug 2016, Stefan Beller wrote: > On Fri, Aug 5, 2016 at 1:20 AM, Johannes Schindelin >wrote: > > > > I also hate to break it to you that git-send-email is not going to be > > part of any solution. > > It's written in perl, so it's not one of the core parts of Git that you > mentioned above. I do use it though for my submission process. The problem is not Perl, but how fiddly it is to set up. And that you lose all the niceties of an email client (e.g. when you want to add a comment before the diff stat that is not intended to become a part of the commit message). But I had an apostrophe last night. I might have been a bit overzealous to claim that git-send-email won't be a part of the solution. It cannot be a *user-visible* part of any solution, that still holds true. So here is the apostrophe: why not implement a bot that listens to the PRs on GitHub, and accepts commands such as "@bot please send this upstream" via comments on the PR. It would then send the patches to the list, from its own email address, on behalf of the contributor. Lots of things to kink out, such as: does it need to be moderated? Record what was submitted in its own git.git fork? Accept replies and attach them to the correct PR? Maybe annotate those replies with the commits whose patches were discussed? Maybe send out replies on the PR as emails? Maybe try to auto-apply suggested patches? Cc: people who commented on earlier iterations of the patch series? Maybe make interaction smarter using an AI bot framework? If only I had lots of time on my hand, I'd start by prototyping a node.js server and hooking it up via webhooks, then show it off so others can tinker with it. This is not completely unlike submitGit, which was a good first attempt, but I never used it because I needed it to do so much more than it already did, *and* it complicated everything by requiring users to register with an extra step to allow submitGit to send email on their behalf. It also made contributing to it harder by choosing some non-standard web app framework. Also, I really do not like having to go to a different website just to send a GitHub PR to the list. Anyway, that was my brain fart for the day. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Philip, On Fri, 5 Aug 2016, Philip Oakley wrote: > In the same vein, I always want,with my workflow, to use "fixup! > ". Good news for you: this is already supported. See here: https://github.com/git/git/blob/v2.9.2/git-rebase--interactive.sh#L786-L796 Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Eric, On Fri, 5 Aug 2016, Eric Wong wrote: > Johannes Schindelinwrote: > > On Thu, 4 Aug 2016, Stefan Beller wrote: > > > git send-email/format-patch recently learned to include a base commit > > > > You may have noticed that my mail-patch-series.sh-generated code > > submissions contain that base commit. But they still do not contain the > > SHA-1s of my local commits corresponding to the patches, and even if they > > did, the replies with suggested edits would most likely have lost said > > information. > > > > I also hate to break it to you that git-send-email is not going to be part > > of any solution. > > I think it ought to be. Some reasons I like emailing patches are: > > [...] What I said is that *git-send-email* is not going to be part of any solution. Note that I said *git-send-email*, not "emailing patches". What many people on this list forget is that few email users *ever* touch their email configuration. Asking them to figure out their SMTP settings and then to make git-send-email work is, uhm, quite a bit unrealistic. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Junio C Hamanowrote: > Somebody mentioned "configuring it is hard--why does the user have > to know SMTP subtleties", and that may be a valid complaint against > the primary part of send-email. The solution for that is not to > discard it with bathwater, but make it just as easy as other MSAs, > say, Thunderbird, to configure for an average user who can configure > these other MUAs. Sadly, the average user does not use an MUA, SMTP or IMAP, anymore. It's all webmail or apps using proprietary protocols. Embrace, extend, extinguish :< -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
From: "Johannes Schindelin"Hi Philip, On Fri, 5 Aug 2016, Philip Oakley wrote: In the same vein, I always want,with my workflow, to use "fixup! ". Good news for you: this is already supported. See here: https://github.com/git/git/blob/v2.9.2/git-rebase--interactive.sh#L786-L796 That's odd , I never saw any of that in the documentation... Blame says it was 68d5d03 (rebase: teach --autosquash to match on sha1 in addition to message, 2010-11-04) which was before I discovered Git. Maybe another documentation fixup needed ;-) Mind you I'm not sure about 22c5b13 (rebase -i: handle fixup! fixup! in --autosquash, 2013-06-27) which looks to only allow one fixup, but maybe I'm misreading. [e.g. recieve multiple fixups from the list, or need extra fixups as code of documentation is tested] The capability is still good to know. Philip -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Fri, Aug 05, 2016 at 05:24:14PM +0200, Johannes Schindelin wrote: [snip] > > > > This "unified review storage format" really does seem to be the missing > > piece. > > FWIW I do not think so. The real trick will be to come up with an > improvement to the process that lets Junio and Peff continue to work as > before, because It Works For Them, while at the same time letting other > people (such as myself) use easy-to-configure tools that add substantial > convenience. > > Which, to me, means that the missing piece is a clever idea how to > integrate with the mail-based process, without requiring everybody and her > dog to switch to a specific mail client. Fair enough, yes it seems to me that git's own review process is probably a separate discussion. As far as review tools such as git-appraise, git-series and git-candidate are concerned, the review storage format really is the missing piece though, in my opinion, at least if we want to live in a world with compatible review tooling. > > > The tool I've been working on for the past year (git-candidate) was > > initially aimed at contrib[1], and was written in perl solely to satisfy > > contrib rules. It would have been python otherwise. > > Oh...? > > $ git ls-files contrib/\*.py | wc -l > 4 > > And for that matter: > > $ git ls-files contrib/\*.go | wc -l > 4 I read this guide[1] before I started, and wanted to be on the safe side. Maybe that was a mistake... :/ > > In fact, there are even PHP scripts: > > $ git ls-files contrib | sed -n 's/.*\.//p' | sort | grep -v '.' | > uniq | tr '\n' ' ' > bash c el Git go perl php pl pm py rst sh tcsh txt zsh > > But again, I do not think that it makes sense to focus too much on a > language, or on a file format, before we came up with a strategy how to > *not* require everybody to change their current ways. Fair enough. :) Thanks, Richard Ipsum [1]: https://www.kernel.org/pub/software/scm/git/docs/howto/new-command.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Johannes Schindelinwrites: > The problem is not Perl, but how fiddly it is to set up. And that you lose > all the niceties of an email client (e.g. when you want to add a comment > before the diff stat that is not intended to become a part of the commit > message). Just this part. I do not think that is fair to send-email. You are blaming its "feature" that allows it to drive format-patch, which I do not consider is the proper part of the command and to which I kept saying from the early days of its introduction that I'd never use it and I think we should discourage its use exactly because it encourages a bad workflow (i.e. you skip the final proof-reading before sending out, and you cannot add footnote comments). Treat it like an MSA just like Thunderbird, just designed to be more suited to send out patches without corruption, and you will be OK. You work, commit and write your message with your favourite editor, do format-patch, reword or add footnote with your favourite editor, and then send it out. You can avoid letting other MSAs that may corrupt whitespaces touch what you will send out if you used send-email, but that is not mandatory. As long as your favourite MSA does not corrupt your message, you can use it. Somebody mentioned "configuring it is hard--why does the user have to know SMTP subtleties", and that may be a valid complaint against the primary part of send-email. The solution for that is not to discard it with bathwater, but make it just as easy as other MSAs, say, Thunderbird, to configure for an average user who can configure these other MUAs. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Stefan Bellerwrote: > On Fri, Aug 5, 2016 at 1:20 AM, Johannes Schindelin > wrote: > > Yet another option would be to have a tool that integrates with the Git > > repository of the Git mailing list represented by public-inbox. > > So my first reaction to that would be: you could push you patches to > that public inbox and it is translated to emails sent on your behalf. > > Which is reinventing submitGit just in a more accessible language? Maybe vger.kernel.org could open its submission (587) port like Debian does. Unfortunately, this hurts the ability to Cc: folks directly and makes vger even more of an SPOF (I guess submitGit also has this problem) Fwiw, I have a public-inbox for my miscellaneous patch barf at s...@80x24.org - https://80x24.org/spew/ http://ou63pmih66umazou.onion/spew/ I will throw up my untested/half-tested patches for various projects so I can still access it across various NATs and firewalls. Using Tor for send-email often works (depending on which blacklists the exit node is on), but is totally optional (you'd expose your IP). ==> ~/bin/spew <== #!/bin/sh exec torsocks git send-email \ --smtp-domain=80x24.org \ --smtp-debug=1 \ --smtp-server-port=submission \ --smtp-server=80x24.org \ --to ${dest-s...@80x24.org} \ --suppress-cc=all \ "$@" All: Feel free to use it for any Free Software projects, too; or better yet, host your own :) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Mon, Sep 17, 2001 at 10:00:00AM +, Stefan Beller wrote: > But both send-email as well as mail-patch-series as well as git-series > are all about the *sending* part. Not about the back and forth part, i.e. > these don't deal with: "here is a fixup on top". And by that I mean > receiving mails and applying them. git-am is there as a front-end > once you obtained the mail, but from what I get, your original problem > is to get up to date with the latest state, that is either in pu or a proposed > fixup mail on top of your series? git-series, at least, is intended to handle the back-and-forth: if you actually publish the series and not just the final result, someone could pull the series, make a (non-fast-forwarding) change to that, make a new series commit, and publish their modified version of your series. You could then incorporate that change. One of the use cases I developed it for was collaborative development of a patch series. (That workflow still needs a lot more tool assistance to become fully usable, not least of which to assist with the process of merging changes to the series. Working on that.) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Johannes Schindelinwrote: > On Thu, 4 Aug 2016, Stefan Beller wrote: > > git send-email/format-patch recently learned to include a base commit > > You may have noticed that my mail-patch-series.sh-generated code > submissions contain that base commit. But they still do not contain the > SHA-1s of my local commits corresponding to the patches, and even if they > did, the replies with suggested edits would most likely have lost said > information. > > I also hate to break it to you that git-send-email is not going to be part > of any solution. I think it ought to be. Some reasons I like emailing patches are: * there's no taking it back once it's sent * it's backed up within seconds by thousands of subscribers :) * doesn't require the reader to have an active connection to fetch out-of-band * doesn't require the reader to be on the same machine capable of cloning/building the project There are times when I've been on a slow machine, or offline when I wanted to read some patches. However, I do like including a pull request in cover letters of a patch series (not necessary for one-offs). But on a side note, I also find it depressing that SMTP is uncompressed and TLS compression is (still?) unsafe. At least I use ssh tunnels w/ compression for IMAP/SMTP to my own server. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
From: "Duy Nguyen"On Wed, Aug 3, 2016 at 6:07 PM, Johannes Schindelin wrote: It would be a totally different matter, of course, if you used the branches I publish via my GitHub repository, added fixup! and squash! commits, published the result to a public repository and then told me to pull from there, that would make things easier. We could even introduce a reword! construct, to make the review of the suggested edits of the commit message easier. On the topic of fixup and squash and everything. Is anyone else annoyed that the commit title is taken for fixup!, squash! instructions? After you have added a few of them, "git log --oneline" becomes useless. All you see is "fixup! A", "fixup! A", "fixup! B", "fixup! A". Would it be better to let the user control the title? We still need the cue "fixup!", "squash!"... at the beginning of the title, but the original commit reference is appended at the end, like s-o-b lines. In the same vein, I always want,with my workflow, to use "fixup! ". This would be to save trying to retype the title correctly, and simply use the abbreviated sha1, which would nicely allow an extra short summaty of what the fixup is about. Philip -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Johannes, On Fri, Aug 5, 2016 at 1:20 AM, Johannes Schindelinwrote: > Hi Stefan, > > On Thu, 4 Aug 2016, Stefan Beller wrote: > >> On Thu, Aug 4, 2016 at 8:58 AM, Johannes Schindelin >> wrote: >> > >> >> If we were to change our workflows drastically, I'd propose to go a >> >> way[1] similar to notedb in Gerrit, or git-series, >> > >> > Gerrit is a huge, non-distributed system. Complex, too. If we change >> > the patch submission process, we should make things *easier*, not >> > *harder*. So I think Gerrit is pretty much out of the question. >> >> I did not propose to use Gerrit or git-series or git appraise. >> >> However whenever I see these projects talking to each other, the talk >> focuses on a "unified review storage format" pretty often, which would >> make them compatible with each other. > > Unless you have a splendid idea how to integrate that unified format with > our review process on the mailing list, I think this makes for a fine > discussion elsewhere. I'd really like to focus on the Git project and its > patch contribution process in this here thread. > >> So then I could choose git-series, while you could go with git appraise >> (although that is written in go, so maybe too fancy already ;) > > I think you misunderstood me in a big way. > > New languages are awesome. I play with new toys whenever I find the time > (and streamlining the contribution process would give me more time for > that). You are talking to a person who implemented the Householder > transformation in Postscript, a 6502 assembler in Forth, and a music > composing system in Emacs Lisp. No language is to fancy for me. "me", as > in "me, personally". > > Now let's think about Git for a moment, and its language choices, and the > rationale behind them. The majority of the critically important code is > written in C. Is C a good language? Decent, yes, but of course also > limiting, Resource leaks are very easy to overlook, and we have a share of > them. No object orientation, so when we need to "subclass", we have no > compile time safety. The pre-processor constructs make static analysis > nigh impossible. Plenty of downsides. So why was it chosen? Developers are > *familiar* with it, that's why. Similar considerations apply to the use of > shell scripts, and to Perl, to a certain extent. Which is changing slowly over time IMHO. The "new generation" of developers may not have in-depth knowledge of shelll or perl any more, but rather python or java maybe. > > I am not talking about contrib/ of course. That's fair game, it contains > only non-critical/fringe stuff. > > Note that the same rationale goes for choosing to accept patch submissions > via mail to a list that is not subscribers-only. > > When it comes to inviting developers to contribute to your project, > personal preferences become irrelevant, the deciding factor becomes how > easy it is to join. Is the language popular, many developers already > familiar with it? Is the build system readily available? Are the > maintainers responsive? I agree. I really do. I had some discussion at lunch yesterday about different attitudes of open source projects towards new contributions. An example was the Eclipse projects that scare off potential new contributors (specially these fly by single patch submissions) as you need to interact through their instance of Gerrit after signing a CLA. Git on the other hand doesn't do a bad job, e.g. I started with a patch that ought to be a single shot drive by patch. About 780 others wrote one patch and were never seen again: $ git shortlog -sne |grep 1 |wc -l 788 So our process is not too bad for the time. However email is as a communication tool is dying [1] eventually? Let's examine if we get less one time contributions over time: for i in $(seq 11 -1 1) ; do x_1=$(git shortlog -sne --since $i.years --until $(($i-1)).years |grep 1 |wc -l) printf "$i\t$x_1\n" done 11 81 10 94 9175 8148 7139 6118 5108 4149 3109 2106 199 Looking at these numbers the number of one time patch contributions spiked 9 years ago and declined since then (4 years ago seems to be an outlier) And I do not think the decline of one off contributions is because Git as a project is in decline, but has other reasons. Maybe the project is in better shape now that there are less one-off worthy contributions? Or the contribution process is not appealing to a lot of new comers? [1] I could not find a scientific paper that evaluates communcation habits of people, but these may give a feel for it: https://www.quora.com/Why-are-young-people-abandoning-email https://news.ycombinator.com/item?id=3554466 http://www.twistimage.com/blog/archives/nobody-uses-email-anymore/ http://www.emailisnotdead.com/ > > I vividly remember my reaction to Darcs, for example. It's written in > Haskell. I am a
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Junio, On Thu, 4 Aug 2016, Junio C Hamano wrote: > Jeff Kingwrites: > > > Like you, I have occasionally been bitten by Junio doing a fixup, and > > then I end up re-rolling, and lose that fixup. > > ... which usually is caught when I receive the reroll, as I try to apply > to the same base and compare the result with the previous round. I find it incredibly nice of you to do those fix-ups, sorry to state that as clearly only now. It's just that I hate to see your time wasted, in particular when *I* waste it because I missed the fix-ups in `pu`. > > But I think such fixups are a calculated risk. Sometimes they save a > > lot of time, both for the maintainer and the contributor, when they > > manage to prevent another round-trip of the patch series to the list. > > Yes. FWIW I am more than just willing to spend a little more time on applying fix-ups and re-rolling patch series (and dual-publishing them via my public repository), if it helps lighten your burden. > > IOW, if the flow is something like: > > > > 1. Contributor sends patches. People review. > > > > 2. Minor fixups noticed by maintainer, fixed while applying. > > This includes different kinds of things: > > a) Trivially correct fixes given in other people's review. > > b) Minor fixups by the maintainer, to code. > > c) Minor fixups by the maintainer, to proposed log message. > > d) "apply --whitespace=fix" whose result I do not even actively >keep track of. > > > 3. Only one small fixup needed from review. Contributor sends > > squashable patch. Maintainer squashes. > > > > then I think that is a net win over sending the whole series again, for > > the contributor (who does not bother sending), reviewers (who really > > only need to look at the interdiff, which is what that squash is in the > > first place), and the maintainer (who can squash just as easily as > > re-applying the whole series). > > > And that is the flip side. If the flow above does not happen, then step > > 2 just becomes a pain. > > I think I can > > * stop taking 2-a). This is less work for me, but some >contributors are leaky and can lose obviously good suggestions, >so I am not sure if that is an overall win for the quality of the >end product; If you had a `git commit --reword` command to touch up commit messages, would that help you, together with the `git commit --fixup` command for code changes? The branches in `pu` would have your fix-ups as strictly separate commits on top of the contributed patches, and the branches would need to be sent through rebase -i before merging to `next`, of course. The idea would be to not forget your fixups in subsequent iterations, but simply rebase them on top of the new iteration. It would still not solve my problem that there is no convenient way to jump from your commits in `pu` to the corresponding ones in my local branch. But that is my problem, not yours. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Richard, On Fri, 5 Aug 2016, Richard Ipsum wrote: > On Thu, Aug 04, 2016 at 09:42:18AM -0700, Stefan Beller wrote: > > On Thu, Aug 4, 2016 at 8:58 AM, Johannes Schindelin > >wrote: > > > > > >> If we were to change our workflows drastically, I'd propose to go a > > >> way[1] similar to notedb in Gerrit, or git-series, > > > > > > Gerrit is a huge, non-distributed system. Complex, too. If we change > > > the patch submission process, we should make things *easier*, not > > > *harder*. So I think Gerrit is pretty much out of the question. > > > > I did not propose to use Gerrit or git-series or git appraise. > > > > However whenever I see these projects talking to each other, the talk > > focuses on a "unified review storage format" pretty often, which would > > make them compatible with each other. So then I could choose > > git-series, while you could go with git appraise (although that is > > written in go, so maybe too fancy already ;) Or there could be another > > new program written in C that follows the "review format". > > This "unified review storage format" really does seem to be the missing > piece. FWIW I do not think so. The real trick will be to come up with an improvement to the process that lets Junio and Peff continue to work as before, because It Works For Them, while at the same time letting other people (such as myself) use easy-to-configure tools that add substantial convenience. Which, to me, means that the missing piece is a clever idea how to integrate with the mail-based process, without requiring everybody and her dog to switch to a specific mail client. > The tool I've been working on for the past year (git-candidate) was > initially aimed at contrib[1], and was written in perl solely to satisfy > contrib rules. It would have been python otherwise. Oh...? $ git ls-files contrib/\*.py | wc -l 4 And for that matter: $ git ls-files contrib/\*.go | wc -l 4 In fact, there are even PHP scripts: $ git ls-files contrib | sed -n 's/.*\.//p' | sort | grep -v '.' | uniq | tr '\n' ' ' bash c el Git go perl php pl pm py rst sh tcsh txt zsh But again, I do not think that it makes sense to focus too much on a language, or on a file format, before we came up with a strategy how to *not* require everybody to change their current ways. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Duy, On Fri, 5 Aug 2016, Duy Nguyen wrote: > On the topic of fixup and squash and everything. Is anyone else > annoyed that the commit title is taken for fixup!, squash! > instructions? I do not know about others, but I am not annoyed by those commit titles. I think they make tons of sense. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Wed, Aug 3, 2016 at 6:07 PM, Johannes Schindelinwrote: > It would be a totally different matter, of course, if you used the > branches I publish via my GitHub repository, added fixup! and squash! > commits, published the result to a public repository and then told me to > pull from there, that would make things easier. We could even introduce a > reword! construct, to make the review of the suggested edits of the commit > message easier. On the topic of fixup and squash and everything. Is anyone else annoyed that the commit title is taken for fixup!, squash! instructions? After you have added a few of them, "git log --oneline" becomes useless. All you see is "fixup! A", "fixup! A", "fixup! B", "fixup! A". Would it be better to let the user control the title? We still need the cue "fixup!", "squash!"... at the beginning of the title, but the original commit reference is appended at the end, like s-o-b lines. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Thu, Aug 04, 2016 at 09:42:18AM -0700, Stefan Beller wrote: > On Thu, Aug 4, 2016 at 8:58 AM, Johannes Schindelin >wrote: > > > >> If we were to change our workflows drastically, I'd propose to > >> go a way[1] similar to notedb in Gerrit, or git-series, > > > > Gerrit is a huge, non-distributed system. Complex, too. If we change the > > patch submission process, we should make things *easier*, not *harder*. So > > I think Gerrit is pretty much out of the question. > > I did not propose to use Gerrit or git-series or git appraise. > > However whenever I see these projects talking to each other, the talk focuses > on > a "unified review storage format" pretty often, which would make them > compatible > with each other. So then I could choose git-series, while you could go with > git appraise (although that is written in go, so maybe too fancy already ;) > Or there could be another new program written in C that follows the "review > format". This "unified review storage format" really does seem to be the missing piece. The tool I've been working on for the past year (git-candidate) was initially aimed at contrib[1], and was written in perl solely to satisfy contrib rules. It would have been python otherwise. The feedback from that thread[1], was that while git-candidate itself seemed interesting it would be unreasonable to bless a particular tool's format. So it seems to me that even if git-series had been written in perl rather than rust it could have expected a similar response to that of git-candidate, possibly. As Stefan says, if we're able to establish a standard for storing review data in git then it doesn't really matter what the tools are written in. For what it's worth my possibly quite shoddy attempt at a library implementing a possible review format for git[2] is written in perl, mostly to satisfy contrib requirements. > > > > Even requiring every contributor to register with GitHub would be too much > > of a limitation, I would wager. > > > > And when I said I have zero interest in tools that use the "latest and > > greatest language", I was hinting at git-series. Rust may be a fine and > > wonderful language. Implementing git-series in Rust, however, immediately > > limited the potential engagement with developers dramatically. Ironically contrib's language requirements actually raised the bar for me because it meant that I had to learn perl. > > > > Additionally, I would like to point out that defining a way to store > > reviews in Git is not necessarily improving the way our code contribution > > process works. If you want to record the discussions revolving around the > > code, I think public-inbox already does a pretty good job at that. I agree, and must apologise if this response has been too off topic, in any case I hope at least some of it was useful to someone. Hope this helps, Richard Ipsum [1]: http://www.mail-archive.com/git%40vger.kernel.org/msg80972.html [2]: https://bitbucket.org/richardipsum/perl-notedb -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Johannes Schindelinwrote: Agreed on all above points :> > On Thu, 4 Aug 2016, Eric Wong wrote: > > Of course, centralized systems are unacceptable to me; > > and with that I'll never claim any network service I run > > will be reliable :) > > Hehehe. I guess that's why the public-inbox is backed by a Git > repository... BTW is it auto-mirrored anywhere? Yep, and the code is AGPL so others can always replicate it. I just have the onions mentioned at the bottom of the HTML pages running something like: torsocks git fetch && public-inbox-index && sleep 30 in a loop[1]. http://hjrcffqmbrq6wope.onion/git http://czquwvybam4bgbro.onion/git I do encourage anybody who is able to, to run their own mirrors (off their existing email subscription) so my MX doesn't become an SPOF, either. I sorta documented it in my original announcement: https://public-inbox.org/git/20160710004813.ga20...@dcvr.yhbt.net/ https://public-inbox.org/INSTALL Feel free to ask me (+ m...@public-inbox.org) for install/running questions related to public-inbox (haven't gotten to making it work outside of Debian stable, though). [1] one of my far-off goals for git be: "git fetch --wait-for-update" to avoid needless wakeups -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Eric, On Thu, 4 Aug 2016, Eric Wong wrote: > Stefan Bellerwrote: > > On Thu, Aug 4, 2016 at 8:58 AM, Johannes Schindelin > > wrote: > > > I guess I have no really good idea yet, either, how to retain the ease of > > > access of sending mails to the list, yet somehow keep a strong tie with > > > the original data stored in Git. > > > > Does it have to be email? Transmitting text could be solved > > differently as well. > > I've thought a lot about this over the years still think email > is the least bad. Not only that: people are *familiar* with it. And they have *access* to it. > Anti-spam tools for other messaging systems are far behind, > proprietary, or non-existent. bugzilla.kernel.org has been hit > hard lately and I see plenty of bug-tracker-to-mail spam as a > result from projects that use web-based bug trackers. Plus, they are all centralized. Do you want to *require* contibutors to register with a new service? > I guess a blockchain (*coin) implementation might work (like > hashcash is used for email anti-spam), but the ones I've glanced > at all look like a bigger waste of electricity than email > filters. I am not even so much concerned with ecological considerations here. Just the price of entry would be prohibitive. > Of course, centralized systems are unacceptable to me; > and with that I'll never claim any network service I run > will be reliable :) Hehehe. I guess that's why the public-inbox is backed by a Git repository... BTW is it auto-mirrored anywhere? Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Stefan, On Thu, 4 Aug 2016, Stefan Beller wrote: > On Thu, Aug 4, 2016 at 8:58 AM, Johannes Schindelin >wrote: > > > >> If we were to change our workflows drastically, I'd propose to go a > >> way[1] similar to notedb in Gerrit, or git-series, > > > > Gerrit is a huge, non-distributed system. Complex, too. If we change > > the patch submission process, we should make things *easier*, not > > *harder*. So I think Gerrit is pretty much out of the question. > > I did not propose to use Gerrit or git-series or git appraise. > > However whenever I see these projects talking to each other, the talk > focuses on a "unified review storage format" pretty often, which would > make them compatible with each other. Unless you have a splendid idea how to integrate that unified format with our review process on the mailing list, I think this makes for a fine discussion elsewhere. I'd really like to focus on the Git project and its patch contribution process in this here thread. > So then I could choose git-series, while you could go with git appraise > (although that is written in go, so maybe too fancy already ;) I think you misunderstood me in a big way. New languages are awesome. I play with new toys whenever I find the time (and streamlining the contribution process would give me more time for that). You are talking to a person who implemented the Householder transformation in Postscript, a 6502 assembler in Forth, and a music composing system in Emacs Lisp. No language is to fancy for me. "me", as in "me, personally". Now let's think about Git for a moment, and its language choices, and the rationale behind them. The majority of the critically important code is written in C. Is C a good language? Decent, yes, but of course also limiting, Resource leaks are very easy to overlook, and we have a share of them. No object orientation, so when we need to "subclass", we have no compile time safety. The pre-processor constructs make static analysis nigh impossible. Plenty of downsides. So why was it chosen? Developers are *familiar* with it, that's why. Similar considerations apply to the use of shell scripts, and to Perl, to a certain extent. I am not talking about contrib/ of course. That's fair game, it contains only non-critical/fringe stuff. Note that the same rationale goes for choosing to accept patch submissions via mail to a list that is not subscribers-only. When it comes to inviting developers to contribute to your project, personal preferences become irrelevant, the deciding factor becomes how easy it is to join. Is the language popular, many developers already familiar with it? Is the build system readily available? Are the maintainers responsive? I vividly remember my reaction to Darcs, for example. It's written in Haskell. I am a mathematician originally, so Haskell appeals to me. Did the choice of language appear to be designed to keep contributors out? To me, it looked that way. Other example: submitGit. I really like what its intention. For a while, I even hoped to move my *own* patch submissions to submitGit. I planned to help get the kinks out of the code by contributing to it. It is written in Scala, using a web application and testing framework I have not encountered elsewhere. I struggled with installing it locally and wrapping my head around the coding paradigms, for 1.5 days (which was all I could afford at that time). Then I had to give up. Which made me very sad. I would not have written my mail-path-series.sh tool if submitGit had been written in node.js, for example, with which I am familiar enough to jump right in. So I hope you understand better now why I find Rust a poor choice for something like git-series, because it should not waste contributors' time by insisting that their price of entry is learning a new language they are unfamiliar with, using a new packaging system, installing a new build setup. I would find Clojure, Crystal or Swift just as poor a choice. Even node.js. It is just too much of a "Keep Out" sign for busy developers. And all the developers worth their salt are busy. > > Additionally, I would like to point out that defining a way to store > > reviews in Git is not necessarily improving the way our code > > contribution process works. If you want to record the discussions > > revolving around the code, I think public-inbox already does a pretty > > good job at that. > > Yeah recording is great, but we want to talk about replying and > modifying a series? So if I see a patch flying by on the mailing list, > ideally I could attach a "!fixup, signed off by Stefan" thing to that > patch. (I said "thing" as I do not necessarily mean email here. Right. I briefly considered suggesting a new tool that would operate on attachments, integrating tightly with the local git.git checkout. Briefly. I had to reject this idea because I do not think that requiring new tools just to contribute to Git would fly well. > > I guess I have no
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Thu, Aug 04, 2016 at 02:12:22PM -0700, Junio C Hamano wrote: > > 2. Minor fixups noticed by maintainer, fixed while applying. > > This includes different kinds of things: > > a) Trivially correct fixes given in other people's review. > > b) Minor fixups by the maintainer, to code. > > c) Minor fixups by the maintainer, to proposed log message. > > d) "apply --whitespace=fix" whose result I do not even actively >keep track of. > > [...] > > I think I can > > * stop taking 2-a). This is less work for me, but some >contributors are leaky and can lose obviously good suggestions, >so I am not sure if that is an overall win for the quality of the >end product; Actually, I think the 2-a class is what often saves a re-roll. Somebody points out a typo in a commit message or a comment, and it quite often gets picked up by you without having another round-trip to the list. If you want to save work by not doing so, that's fine with me. But this is the gamble I was talking about. I think it's actually often less work to do the fixup than to look at another re-roll (especially with the "leaky contributor" thing where you have to make sure all fixes were applied). So it's a win if it saves the re-roll, but sometimes you end up having to look at the re-roll anyway. > * do a separate "SQUASH???" commit and send them out for 2-b). >This is a lot more work for a large series, but about the same >amount of work (except for "remembering to send them out" part) >for a small-ish topic. A contributor needs to realize that I >deal with _all_ the incoming topics, not just from topics from >one contributor, and small additional work tends to add up. I think these are largely the same as 2-a. You are just wearing two hats, reviewer and maintainer. Which I guess lets you take a shortcut sometimes (and just fix without mentioning it), but fundamentally the "gamble" aspect is the same, I think. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Jeff Kingwrites: > Like you, I have occasionally been bitten by Junio doing a fixup, and > then I end up re-rolling, and lose that fixup. ... which usually is caught when I receive the reroll, as I try to apply to the same base and compare the result with the previous round. > But I think such fixups are a calculated risk. Sometimes they save a lot > of time, both for the maintainer and the contributor, when they manage > to prevent another round-trip of the patch series to the list. Yes. > IOW, if the flow is something like: > > 1. Contributor sends patches. People review. > > 2. Minor fixups noticed by maintainer, fixed while applying. This includes different kinds of things: a) Trivially correct fixes given in other people's review. b) Minor fixups by the maintainer, to code. c) Minor fixups by the maintainer, to proposed log message. d) "apply --whitespace=fix" whose result I do not even actively keep track of. > 3. Only one small fixup needed from review. Contributor sends > squashable patch. Maintainer squashes. > > then I think that is a net win over sending the whole series again, for > the contributor (who does not bother sending), reviewers (who really > only need to look at the interdiff, which is what that squash is in the > first place), and the maintainer (who can squash just as easily as > re-applying the whole series). > And that is the flip side. If the flow above does not happen, then step > 2 just becomes a pain. I think I can * stop taking 2-a). This is less work for me, but some contributors are leaky and can lose obviously good suggestions, so I am not sure if that is an overall win for the quality of the end product; * do a separate "SQUASH???" commit and send them out for 2-b). This is a lot more work for a large series, but about the same amount of work (except for "remembering to send them out" part) for a small-ish topic. A contributor needs to realize that I deal with _all_ the incoming topics, not just from topics from one contributor, and small additional work tends to add up. to reduce #2. Essentially, doing these two are about moving more responsibility of keeping track of good suggestions in the review discussion to the contributor, but a bad thing is that it does not mean that the maintainer can stop keeping track of them. We need a way to make sure leaky contributors do not repeat the same issue in their reroll that has already been pointed out and whose solutions presented in the previous review. Fetching from 'pu' and compare has been one way to do so (that is why I publish 'pu' in the first place, not to "build on", but to "view"), but apparently not many contributors are comfortable with that idea. There is no good way to reduce 2-c) and 2-d), but on the other hand, there is no strong need for a special channel to propagate these back. 2-c) can be a regular review comment (but again that risks "the leaky contributor" problem) and 2-d) will happen automatically when applying the rerolled version. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Stefan Bellerwrote: > On Thu, Aug 4, 2016 at 8:58 AM, Johannes Schindelin > wrote: > > I guess I have no really good idea yet, either, how to retain the ease of > > access of sending mails to the list, yet somehow keep a strong tie with > > the original data stored in Git. > > Does it have to be email? Transmitting text could be solved > differently as well. I've thought a lot about this over the years still think email is the least bad. Anti-spam tools for other messaging systems are far behind, proprietary, or non-existent. bugzilla.kernel.org has been hit hard lately and I see plenty of bug-tracker-to-mail spam as a result from projects that use web-based bug trackers. And email spam filtering isn't even that great (and I think it needs to be better for IPv6 and .onion adoption since much of it is still IPv4-oriented blacklisting). I guess a blockchain (*coin) implementation might work (like hashcash is used for email anti-spam), but the ones I've glanced at all look like a bigger waste of electricity than email filters. Of course, centralized systems are unacceptable to me; and with that I'll never claim any network service I run will be reliable :) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Thu, Aug 04, 2016 at 05:29:52PM +0200, Johannes Schindelin wrote: > So my idea was to introduce a new --reword= option to `git commit` > that would commit an empty patch and let the user edit the commit message, > later replacing the original one with the new one. This is not *quite* as > nice as I want it, because it makes the changes unobvious. On the other > hand, I would not find a series of sed commands more obvious, in > particular because that limits you in the ways of sed. And, you know, > regexp. I like them, but I know many people cannot really work with them. I don't have a real opinion on this. I probably wouldn't use it, but I have no problem with it existing. I think it's somewhat orthogonal to the idea of _transmitting_ those reword operations to somebody else. > > That pushes work onto the submitter, but saves work from the reviewers, > > who can quickly say "something like this..." without having to worry > > about making a full change, formatting it as a diff, etc. > > > > I do think that's the right time-tradeoff to be making, as we have more > > submitters than reviewers. > > I agree that it is the right trade-off. TBH I was shocked when I learned > how much effort Junio puts into applying my patches. I do not want that. I > want my branch to reflect pretty precisely (modulo sign-off, of course) > what is going to be integrated into Git's source code. Like you, I have occasionally been bitten by Junio doing a fixup, and then I end up re-rolling, and lose that fixup (or have to deal with porting it forward with awkward tools). But I think such fixups are a calculated risk. Sometimes they save a lot of time, both for the maintainer and the contributor, when they manage to prevent another round-trip of the patch series to the list. IOW, if the flow is something like: 1. Contributor sends patches. People review. 2. Minor fixups noticed by maintainer, fixed while applying. 3. Only one small fixup needed from review. Contributor sends squashable patch. Maintainer squashes. then I think that is a net win over sending the whole series again, for the contributor (who does not bother sending), reviewers (who really only need to look at the interdiff, which is what that squash is in the first place), and the maintainer (who can squash just as easily as re-applying the whole series). It does mean the "final" version of the series is never on the list. It has to be pieced together from the squash (and sometimes step 2 is not even mentioned on-list). So I think it is really a judgement call for step (3) on what is a "small" fixup, and whether it is easier for everybody to look at the squash interdiff and say "yep, that's right", versus re-reviewing the whole series. > I'd much prefer to resubmit a cleaned-up version, even if it was just the > commit subjects, and be certain that `pu` and my branch are on the same > page. > > Instead, Junio puts in a tremendous amount of work, and it does not help > anybody, because the local branches *still* do not have his fixups, and as > a consequence subsequent iterations of the patch series will have to be > fixed up *again*. And that is the flip side. If the flow above does not happen, then step 2 just becomes a pain. I don't have a silver bullet or anything. I'm mostly just musing. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Thu, Aug 4, 2016 at 8:58 AM, Johannes Schindelinwrote: > >> If we were to change our workflows drastically, I'd propose to >> go a way[1] similar to notedb in Gerrit, or git-series, > > Gerrit is a huge, non-distributed system. Complex, too. If we change the > patch submission process, we should make things *easier*, not *harder*. So > I think Gerrit is pretty much out of the question. I did not propose to use Gerrit or git-series or git appraise. However whenever I see these projects talking to each other, the talk focuses on a "unified review storage format" pretty often, which would make them compatible with each other. So then I could choose git-series, while you could go with git appraise (although that is written in go, so maybe too fancy already ;) Or there could be another new program written in C that follows the "review format". > > Even requiring every contributor to register with GitHub would be too much > of a limitation, I would wager. > > And when I said I have zero interest in tools that use the "latest and > greatest language", I was hinting at git-series. Rust may be a fine and > wonderful language. Implementing git-series in Rust, however, immediately > limited the potential engagement with developers dramatically. > > Additionally, I would like to point out that defining a way to store > reviews in Git is not necessarily improving the way our code contribution > process works. If you want to record the discussions revolving around the > code, I think public-inbox already does a pretty good job at that. Yeah recording is great, but we want to talk about replying and modifying a series? So if I see a patch flying by on the mailing list, ideally I could attach a "!fixup, signed off by Stefan" thing to that patch. (I said "thing" as I do not necessarily mean email here. > > I guess I have no really good idea yet, either, how to retain the ease of > access of sending mails to the list, yet somehow keep a strong tie with > the original data stored in Git. Does it have to be email? Transmitting text could be solved differently as well. With git push/fetch we can interact with a git remote and pertain the state (commits, ancestor graph) at a full level even including notes that comment on commits. git send-email/format-patch recently learned to include a base commit (xy/format-patch-base), maybe we need a counter part to git send-email that downloads a series from your mailbox, such that a local branch can be transmitted to via "git send-email --base=origin/master --include-notes --name=sb/new-series" and completely reconstructed (i.e. the commit sha1s even match) including notes via: git fetch-email --name=sb/new-series That way would ensure we have a "simple" way to transmit patches back and forth and adding potential fixups. You wrote: > In short, I agree that our patch submission process is a saber tooth tiger > that still reflects pre-Git times. While we use Git's tools, the workflow > really tries to cut out Git as much as possible, in favor of pure mails > with non-corrupted, non-HTML patches in them, a charmingly anachronistic > requirement until you try to use state-of-the-art mail clients to send > them. And there are two ways out: * either we teach git how to deal with emails (completely, i.e. sending+receiving) * or we change the development model (e.g. no emails any more) There is no golden third way IMHO. Thanks, Stefan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Stefan, On Wed, 3 Aug 2016, Stefan Beller wrote: > On Wed, Aug 3, 2016 at 9:07 AM, Johannes Schindelin >wrote: > > > > On Wed, 3 Aug 2016, Junio C Hamano wrote: > > > >> On Wed, Aug 3, 2016 at 4:59 AM, Johannes Schindelin > >> wrote: > >> > > >> > I disagree, however, with the suggestion to sift through your `pu` > >> > branch and to somehow replace local branches with the commits found > >> > there. > >> > >> To be more in line with the "e-mailed patch" workflow, I think what I > >> should do is to send the version I queued with fixups back to the > >> list as follow-up. Just like reviewers review, the maintainer > >> reviews and queues, the original author should be able to work in the > >> same workflow, i.e. reading and applying an improved version of the > >> patch from her mailbox. > > > > You seem to assume that it isn't cumbersome for people like me to > > extract patches out of mails and to replace existing commits using > > those patches. > > > > So it probably comes as a huge surprise to you to learn that this *is* > > cumbersome for me. > > It is also cumbersome for me, because I never had the need to setup a > proper mail client that has the strength to apply patches. The need was > not there as I tend to apply only rarely patches by email, so I can go > the painful way each time. The reason is clear, too. Mail clients serve humans. That is their purpose. Humans do not care all that much whether the text was preserved exactly as the sender wrote it, except rich text (read: HTML), of course. > > I got too used to the ease of git push, git pull with or without > > --rebase, and many other Git commands. Having to transmogrify code > > changes from commits in Git into a completely different universe: > > plain text patches in my mailbox, and back, losing all kinds of data > > in the process, is just not at all that easy. And it costs a lot of > > time. > > > > In short: if you start "submitting patches" back to me via mail, it > > does not help me. It makes things harder for me. In particular when > > you add your sign-off to every patch and I have to strip it. > > You don't have to strip the sign off, as it shows the flow of the patch, > e.g. > > Signed-off-by: Johannes Schindelin > Signed-off-by: Junio C Hamano > Signed-off-by: Stefan Beller > Signed-off-by: Johannes Schindelin > Signed-off-by: Junio C Hamano > > may indicate you proposed a patch, Junio picked it up (and fixed a typo > optionally), I obtained the patch (via mail, via Git?) improved it, you > improved it further and then Junio took it and merged it upstream. Recently, I got yelled at because I took one of Junio's patches, made a couple of changes, and *added* my sign-off. Before that incident, I agreed with you that it may make for a nice record of the back-and-forth that eventually resulted in the patch in question. Now, I am not so sure anymore. > > If you change your workflow, I would humbly request that you do it in > > a way that makes things easier on both sides, not harder. > > When attending the Git Merge conference in May, gregkh said roughtly: > "We deliberately waste developers time, because it is not scarce. > Maintainers time is scarce however " and it stuck with me. (and I am a > developer, not a maintainer ;( so at least the kernel community deems it > ok to waste my time). Yeah. It was not the only thing I disagreed with in his talk. To be a little bit blunt (by my standards, not by LKML's standards, that is): the Linux kernel mailing list is not necessarily anything I would want to use as a role model. I agree that maintainers' time is scarce. I am one. So of course I agree with that statement. What I disagree with is that it is okay to *waste* contributors' time. That's just inconsiderate. And I say that also because I am a contributor *in addition* to being a maintainer. As a consequence, I commend Greg for recognizing that the patch submission process must be light on the maintainer. And I would have commended him even further if he had realized that proper tooling should waste nothing, and no one's time. > While that is true for the kernel community, I guess it is also true for > the Git community, unless Junio (and the community) want to appoint a > bunch of maintainer lieutenants, such that they outnumber the number of > developers, e.g. divided by areas of the code: a refs backend > maintainer, a submodule maintainer, ... or rather by area of usage: a > porcelain UI maintainer, a git-on-server maintainer. As I mentioned earlier, I do not care much about following LKML's example. What I see on this here list is that many a potential contributor is scared away, that we waste precious time (also the maintainer's) pointing out in what way certain contributions do not follow the guide lines, and that even
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Peff, On Wed, 3 Aug 2016, Jeff King wrote: > On Wed, Aug 03, 2016 at 09:53:18AM -0700, Junio C Hamano wrote: > > > > Leaving aside Dscho's questions of whether pulling patches from email is > > > convenient for most submitters (it certainly is for me, but I recognize > > > that it is not for many), I would much rather see incremental fixup > > > patches from you than whole "here's what I queued" responses. > > > > Ah, yes, I misspoke. It should be either an incremental diff or > > in-line comment to spell out what got changed as a response to the > > patch. > > > > I find myself fixing the title the most often, which is part of the > > "log message" you pointed out that would not convey well with the > > "incremental diff" approach. > > I mentioned a micro-format elsewhere in my message. And it certainly is > nice to have something that can be applied in an automatic way. Indeed. This is what I meant by my (succinct to the point of being intelligible, admittedly) reword! suggestion. Let's clarify this idea. I find myself using fixup! and squash! commits a lot. Actually, let me pull out the Linux key for that. I use those commits A LOT. I know, I opposed the introduction of this feature initially (and I think that my concerns were nicely addressed by Junio's suggestion to guard this feature behind the --autosquash option). Guess what: I was wrong. And I am really missing the same functionality for the commit message munging. These days, I find myself using `git commit --allow-empty --squash=$COMMIT -c $COMMIT` very often, duplicating the first line, adding an empty line between them, deleting the "squash! " prefix from the now-third line, and then editing the commit message as I want to. When it comes to cleaning up the branch via rebase -ki, I simply jump to the empty line after the squash! line and delete everything before it. This is as repetitive, tedious and un-fun to me as having to transmogrify patches from the nice and cozy Git universe into the not-at-all compatible universe of mails (I congratulate you personally, Peff, for finding a mail client that works for you. I am still looking for one that does not suck, Alpine being the least sucky I settled for). So my idea was to introduce a new --reword= option to `git commit` that would commit an empty patch and let the user edit the commit message, later replacing the original one with the new one. This is not *quite* as nice as I want it, because it makes the changes unobvious. On the other hand, I would not find a series of sed commands more obvious, in particular because that limits you in the ways of sed. And, you know, regexp. I like them, but I know many people cannot really work with them. > But in practice, most review comments, for the commit message _or_ the > text, are given in human-readable terms. And as a human, I read and > apply them in sequence. So true. I do the very same. > That pushes work onto the submitter, but saves work from the reviewers, > who can quickly say "something like this..." without having to worry > about making a full change, formatting it as a diff, etc. > > I do think that's the right time-tradeoff to be making, as we have more > submitters than reviewers. I agree that it is the right trade-off. TBH I was shocked when I learned how much effort Junio puts into applying my patches. I do not want that. I want my branch to reflect pretty precisely (modulo sign-off, of course) what is going to be integrated into Git's source code. I'd much prefer to resubmit a cleaned-up version, even if it was just the commit subjects, and be certain that `pu` and my branch are on the same page. Instead, Junio puts in a tremendous amount of work, and it does not help anybody, because the local branches *still* do not have his fixups, and as a consequence subsequent iterations of the patch series will have to be fixed up *again*. Just compare https://github.com/git/git/compare/1fd7e78...6999bc7 to https://github.com/dscho/git/compare/f8f7adc...3b4494c (the onelines are enough to show you just how different things are). I'd much prefer the contributor (me, in this case) to put in a little more work, and have things consistent. And avoid unnecessary work on both sides. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Wed, Aug 3, 2016 at 9:07 AM, Johannes Schindelinwrote: > Hi Junio, > > On Wed, 3 Aug 2016, Junio C Hamano wrote: > >> On Wed, Aug 3, 2016 at 4:59 AM, Johannes Schindelin >> wrote: >> > >> > I disagree, however, with the suggestion to sift through your `pu` branch >> > and to somehow replace local branches with the commits found there. >> >> To be more in line with the "e-mailed patch" workflow, I think what I should >> do is to send the version I queued with fixups back to the list as follow-up. >> Just like reviewers review, the maintainer reviews and queues, the original >> author should be able to work in the same workflow, i.e. reading and applying >> an improved version of the patch from her mailbox. > > You seem to assume that it isn't cumbersome for people like me to extract > patches out of mails and to replace existing commits using those patches. > > So it probably comes as a huge surprise to you to learn that this *is* > cumbersome for me. It is also cumbersome for me, because I never had the need to setup a proper mail client that has the strength to apply patches. The need was not there as I tend to apply only rarely patches by email, so I can go the painful way each time. But if we as a community decide that we bounce emails back and forth, I (and you) may have to find a proper email client that is easy to work with, e.g. one key shortcut to apply a patch series to the HEAD of your local repository. > > I got too used to the ease of git push, git pull with or without --rebase, > and many other Git commands. Having to transmogrify code changes from > commits in Git into a completely different universe: plain text patches in > my mailbox, and back, losing all kinds of data in the process, is just > not at all that easy. And it costs a lot of time. > > In short: if you start "submitting patches" back to me via mail, it does > not help me. It makes things harder for me. In particular when you add > your sign-off to every patch and I have to strip it. You don't have to strip the sign off, as it shows the flow of the patch, e.g. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano Signed-off-by: Stefan Beller Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano may indicate you proposed a patch, Junio picked it up (and fixed a typo optionally), I obtained the patch (via mail, via Git?) improved it, you improved it further and then Junio took it and merged it upstream. > > If you change your workflow, I would humbly request that you do it in a > way that makes things easier on both sides, not harder. When attending the Git Merge conference in May, gregkh said roughtly: "We deliberately waste developers time, because it is not scarce. Maintainers time is scarce however " and it stuck with me. (and I am a developer, not a maintainer ;( so at least the kernel community deems it ok to waste my time). While that is true for the kernel community, I guess it is also true for the Git community, unless Junio (and the community) want to appoint a bunch of maintainer lieutenants, such that they outnumber the number of developers, e.g. divided by areas of the code: a refs backend maintainer, a submodule maintainer, ... or rather by area of usage: a porcelain UI maintainer, a git-on-server maintainer. Though Git is not as diverse and large as the kernel, so the horde of maintainers would step onto each feet quite frequently IMHO. > > It would be a totally different matter, of course, if you used the > branches I publish via my GitHub repository, added fixup! and squash! > commits, published the result to a public repository and then told me to > pull from there, that would make things easier. We could even introduce a > reword! construct, to make the review of the suggested edits of the commit > message easier. I could easily verify that my branch head agrees with the > base commit of your branch, I could build proper tooling around this > workflow, and it would lighten my load. > > I guess what I am saying is that we might just as well start using this > awesome tool to work with code, that tool named "Git". I think Git itself is for the tracking the code and managing it, e.g. merging, moving, keeping it. That doesn't quite include modifying and creating code (e.g. there is no "git edit" command) If we were to change our workflows drastically, I'd propose to go a way[1] similar to notedb in Gerrit, or git-series, which defines a common review format, such that we have a "protocol" how to store the review data and how to store the progress of potential collaboration and then we can develop tools against that protocol. Some people want to have a web UI, whereas others want to have a text only thing as they are faster keyboard only. [1]
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Jeff Kingwrites: > On Wed, Aug 03, 2016 at 08:33:12AM -0700, Junio C Hamano wrote: > >> On Wed, Aug 3, 2016 at 4:59 AM, Johannes Schindelin >> wrote: >> > >> > I disagree, however, with the suggestion to sift through your `pu` branch >> > and to somehow replace local branches with the commits found there. >> >> To be more in line with the "e-mailed patch" workflow, I think what I should >> do is to send the version I queued with fixups back to the list as follow-up. >> Just like reviewers review, the maintainer reviews and queues, the original >> author should be able to work in the same workflow, i.e. reading and applying >> an improved version of the patch from her mailbox. > > Leaving aside Dscho's questions of whether pulling patches from email is > convenient for most submitters (it certainly is for me, but I recognize > that it is not for many), I would much rather see incremental fixup > patches from you than whole "here's what I queued" responses. Ah, yes, I misspoke. It should be either an incremental diff or in-line comment to spell out what got changed as a response to the patch. I find myself fixing the title the most often, which is part of the "log message" you pointed out that would not convey well with the "incremental diff" approach. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Wed, Aug 03, 2016 at 09:53:18AM -0700, Junio C Hamano wrote: > > Leaving aside Dscho's questions of whether pulling patches from email is > > convenient for most submitters (it certainly is for me, but I recognize > > that it is not for many), I would much rather see incremental fixup > > patches from you than whole "here's what I queued" responses. > > Ah, yes, I misspoke. It should be either an incremental diff or > in-line comment to spell out what got changed as a response to the > patch. > > I find myself fixing the title the most often, which is part of the > "log message" you pointed out that would not convey well with the > "incremental diff" approach. I mentioned a micro-format elsewhere in my message. And it certainly is nice to have something that can be applied in an automatic way. But in practice, most review comments, for the commit message _or_ the text, are given in human-readable terms. And as a human, I read and apply them in sequence. That pushes work onto the submitter, but saves work from the reviewers, who can quickly say "something like this..." without having to worry about making a full change, formatting it as a diff, etc. I do think that's the right time-tradeoff to be making, as we have more submitters than reviewers. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Wed, Aug 03, 2016 at 08:33:12AM -0700, Junio C Hamano wrote: > On Wed, Aug 3, 2016 at 4:59 AM, Johannes Schindelin >wrote: > > > > I disagree, however, with the suggestion to sift through your `pu` branch > > and to somehow replace local branches with the commits found there. > > To be more in line with the "e-mailed patch" workflow, I think what I should > do is to send the version I queued with fixups back to the list as follow-up. > Just like reviewers review, the maintainer reviews and queues, the original > author should be able to work in the same workflow, i.e. reading and applying > an improved version of the patch from her mailbox. Leaving aside Dscho's questions of whether pulling patches from email is convenient for most submitters (it certainly is for me, but I recognize that it is not for many), I would much rather see incremental fixup patches from you than whole "here's what I queued" responses. The reason is that your fixups may not be the only ones needed. There may be others on the list that come before or after, and I may even have already made fixes locally for "v2" that haven't been on the list. If I haven't made any changes yet, I can throw out my topic, start with what you queued, and then apply other changes incrementally. But if I have, then I need to convert yours to a diff, which requires checking out the same base, applying yours, and running diff. Much easier to get the diff in the first place. :) That only covers changes to the code, though. It does not help with fixups to commit messages. It would be neat to have a microformat for specifying and applying patches to commit messages. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Junio, On Wed, 3 Aug 2016, Junio C Hamano wrote: > On Wed, Aug 3, 2016 at 4:59 AM, Johannes Schindelin >wrote: > > > > I disagree, however, with the suggestion to sift through your `pu` branch > > and to somehow replace local branches with the commits found there. > > To be more in line with the "e-mailed patch" workflow, I think what I should > do is to send the version I queued with fixups back to the list as follow-up. > Just like reviewers review, the maintainer reviews and queues, the original > author should be able to work in the same workflow, i.e. reading and applying > an improved version of the patch from her mailbox. You seem to assume that it isn't cumbersome for people like me to extract patches out of mails and to replace existing commits using those patches. So it probably comes as a huge surprise to you to learn that this *is* cumbersome for me. I got too used to the ease of git push, git pull with or without --rebase, and many other Git commands. Having to transmogrify code changes from commits in Git into a completely different universe: plain text patches in my mailbox, and back, losing all kinds of data in the process, is just not at all that easy. And it costs a lot of time. In short: if you start "submitting patches" back to me via mail, it does not help me. It makes things harder for me. In particular when you add your sign-off to every patch and I have to strip it. If you change your workflow, I would humbly request that you do it in a way that makes things easier on both sides, not harder. It would be a totally different matter, of course, if you used the branches I publish via my GitHub repository, added fixup! and squash! commits, published the result to a public repository and then told me to pull from there, that would make things easier. We could even introduce a reword! construct, to make the review of the suggested edits of the commit message easier. I could easily verify that my branch head agrees with the base commit of your branch, I could build proper tooling around this workflow, and it would lighten my load. I guess what I am saying is that we might just as well start using this awesome tool to work with code, that tool named "Git". Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
On Wed, Aug 3, 2016 at 4:59 AM, Johannes Schindelinwrote: > > I disagree, however, with the suggestion to sift through your `pu` branch > and to somehow replace local branches with the commits found there. To be more in line with the "e-mailed patch" workflow, I think what I should do is to send the version I queued with fixups back to the list as follow-up. Just like reviewers review, the maintainer reviews and queues, the original author should be able to work in the same workflow, i.e. reading and applying an improved version of the patch from her mailbox. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Junio, On Tue, 2 Aug 2016, Junio C Hamano wrote: > So either I should change my workflow and mention any and all > typofixes in my review comments (which consumes the review > bandwidth), or I should force patch authors to do the "fetch from > 'pu' and replace" somehow to avoid this kind of back-and-forth. It never occurred to me that I should replace any of my local branches with versions you have in `pu`. For several reasons: - just as you do not pull from me, lest patches enter your repository that you have not reviewed, I do not simply pull from you, - I thought the point of avoiding GitHub in the review process at all costs and require everybody to go via the mailing list instead was to keep the review process open? These silent changes fly in the face of that, - I agree that the mail-based process is cumbersome and error-prone, but we won't fix it by asking contributors to dig through the `pu` of the day, somehow stay up-to-date with possibly more silent typofixes the next day, somehow manage to figure out what changes exactly were introduced to their patches, and replace their local work. - even if we asked for all that trouble, the commits in `pu` are all signed off by you. These sign-offs would have to be stripped out tediously when making local changes. In short, I agree that our patch submission process is a saber tooth tiger that still reflects pre-Git times. While we use Git's tools, the workflow really tries to cut out Git as much as possible, in favor of pure mails with non-corrupted, non-HTML patches in them, a charmingly anachronistic requirement until you try to use state-of-the-art mail clients to send them. I disagree, however, with the suggestion to sift through your `pu` branch and to somehow replace local branches with the commits found there. I guess it is time to revisit our patch submission process if it does not even work between the two of us. Ideally, we would come up with a process that - makes everything easier for maintainers and contributors alike, - tracks the history of the patch iterations (answering the question "what changed between iterations?"), - *actually* integrates with Git (to see what I mean, try to find the commit corresponding to a given mail containing a patch, and then try to find the previous iteration's version of the same commit, and weep), - provides machine-readable metadata about the context, e.g. to jump back and forth between the full file contents and the patch, or to indicate the dependency on another branch, - facilitates "back contributions", i.e. letting contributors accept changes suggested by reviewers *with minimal effort*. - uses Git itself as much as possible, i.e. no additional tools written in "you must learn this new language, it's awesome, believe me, it's huge" The biggest obstacles I see are 1) the integration with the mailing list (which is ironic because contributing via the list used to be a boon, not a burden) and 2) maintaining the integrity between what has been reviewed and what is actually in the branch. This is nothing we will solve overnight, of course. But I think we will have to fix this. Food for thought. Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Johannes Schindelinwrites: > I tend to think that the underscore is correct: this change is not so much > about the builtin (which is written with a dash) but about the function > (written with an underscore, used by more than just merge-recursive, e.g. > cherry-pick). Yes, I agree. "merge-recursive:" prefix is about either the built-in command, or the machinery as a whole to support that built-in command. It is preferrable to use "merge_recursive():" if we are talking about a single function. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Johannes Schindelinwrites: >> > There are a couple of places where return values never indicated errors >> > before, as wie simply died instead of returning. >> >> s/wie/we/; > > Right. What can I say, I am surrounded by too many Germans. > > I fixed this locally, in case another re-roll should be required. What you > have in `pu` looks correct to me, though. Let me know if you want me to > re-submit nevertheless. I usually do this kind of obvious typofix and consistency fix without even mentioning them in my review comments to reduce the noise levels. But that works better ONLY if the patch authors then fetch from 'pu' and replace their copies with what I fixed up already and base their reroll on top by amending and/or building on top (of course, that also requires my local fix must all be limited to uncontroversial ones). So either I should change my workflow and mention any and all typofixes in my review comments (which consumes the review bandwidth), or I should force patch authors to do the "fetch from 'pu' and replace" somehow to avoid this kind of back-and-forth. I am not sure which should be the way to go. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Hi Junio, On Mon, 1 Aug 2016, Junio C Hamano wrote: > Johannes Schindelin <johannes.schinde...@gmx.de> writes: > > > Subject: Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors > > s/merge_/merge-/; for this one alone. I see that de8946de1694a8cf311daab7b2c416d76cb04d23 still shows it with an underscore instead of a dash, while you fixed my other tyops in this patch series. I tend to think that the underscore is correct: this change is not so much about the builtin (which is written with a dash) but about the function (written with an underscore, used by more than just merge-recursive, e.g. cherry-pick). > > There are a couple of places where return values never indicated errors > > before, as wie simply died instead of returning. > > s/wie/we/; Right. What can I say, I am surrounded by too many Germans. I fixed this locally, in case another re-roll should be required. What you have in `pu` looks correct to me, though. Let me know if you want me to re-submit nevertheless. BTW I should have said this earlier: I run all my rebases, all my merges, all my cherry-picks using a Git version with these patches for months (of course, the patches have changed, but not in the most critical parts I was concerned about, the parts where die() calls were replaced). If I would have found any regression, I would have notified you immediately, of course. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors
Johannes Schindelin <johannes.schinde...@gmx.de> writes: > Subject: Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors s/merge_/merge-/; for this one alone. > There are a couple of places where return values never indicated errors > before, as wie simply died instead of returning. s/wie/we/; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 06/16] merge_recursive: abort properly upon errors
There are a couple of places where return values never indicated errors before, as wie simply died instead of returning. But now negative return values mean that there was an error and we have to abort the operation. Let's do exactly that. Signed-off-by: Johannes Schindelin--- merge-recursive.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 3a652b7..58ced25 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1949,17 +1949,19 @@ int merge_recursive(struct merge_options *o, /* * When the merge fails, the result contains files * with conflict markers. The cleanness flag is -* ignored, it was never actually used, as result of -* merge_trees has always overwritten it: the committed -* "conflicts" were already resolved. +* ignored (unless indicating an error), it was never +* actually used, as result of merge_trees has always +* overwritten it: the committed "conflicts" were +* already resolved. */ discard_cache(); saved_b1 = o->branch1; saved_b2 = o->branch2; o->branch1 = "Temporary merge branch 1"; o->branch2 = "Temporary merge branch 2"; - merge_recursive(o, merged_common_ancestors, iter->item, - NULL, _common_ancestors); + if (merge_recursive(o, merged_common_ancestors, iter->item, + NULL, _common_ancestors) < 0) + return -1; o->branch1 = saved_b1; o->branch2 = saved_b2; o->call_depth--; @@ -1975,6 +1977,8 @@ int merge_recursive(struct merge_options *o, o->ancestor = "merged common ancestors"; clean = merge_trees(o, h1->tree, h2->tree, merged_common_ancestors->tree, ); + if (clean < 0) + return clean; if (o->call_depth) { *result = make_virtual_commit(mrtree, "merged tree"); @@ -2031,6 +2035,9 @@ int merge_recursive_generic(struct merge_options *o, hold_locked_index(lock, 1); clean = merge_recursive(o, head_commit, next_commit, ca, result); + if (clean < 0) + return clean; + if (active_cache_changed && write_locked_index(_index, lock, COMMIT_LOCK)) return error(_("Unable to write index.")); -- 2.9.0.281.g286a8d9 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html