Vim development process is utterly broken (Was: Re: [patch] test98 stops when using gvim)

[Lech Lorens]

On 14-Jul-2013 Bram Moolenaar <Br...@moolenaar.net> wrote:
>
> Ken Takata wrote:
>
> > When I run the test 98 with gvim, it stops at the last line of test98a.in.
> >
> > > call feedkeys(":setl scb\n\<C-w>\<C-w>", 't')
> >
> > It seems that feedkeys() works different on CUI and GUI.
> > I don't know why the difference occurs, but using feedkeys()
> > is not necessary for this test. I think using "setl scb" and
> > "wincmd w" is enough.
> > Please check the attached patch.
>
> Thanks. I also have no clue why feedkeys() was used here.

Ken: sorry for including you here, even thought only a tiny bit is
a reply for you.

I have been thinking about writing this email for at least a year now.
This patch is the straw that breaks the camel's back, so here it comes.

feedkeys() is essential there.
Try re-running the test without the fix that was introduced along with
the test. Magic: the test succeeds even without the fix!
It took 9 days for the fix to be accepted for inclusion because you,
Bram, asked for a working test (one that would fail without the fix).
Now it takes 40 minutes (!!!) to have a patch included that breaks the
test and makes all my effort (I wrote how I needed to use gdb to come up
with a method that would reproduce the problem in a script) go to
a waste.

I know, my bad for not putting in a comment, but the patches are
micromanaged to the point that the name of the file test96extra.in gets
renamed to test98a.in yet the very point of tests gets lost after only
4 days (!!!) since inclusion! That's less time than it took me to
prepare the bloody test!

Bram, don't get offended, I actually do hope that one day I will become as
competent a programmer as you are, but still I think Vim's development
process is severely broken:
- the single point of failure (Bram),
- how the decision what gets included is made with the community totally
oblivious,
- we don't have a development branch of Vim because we want stability.
Therefore the unaware users are without a single warning presented
with 7.3.970 which breaks half of syntax scripts, makes the rest 10
times slower, breaks regular expressions and introduces random
crashes. Only after a number of patches and becoming stable is the
only branch renamed to 7.4 alpha!
- we manage patches as files (as opposed to the distributed VCS way:
pull requests) because it allegedly "just works". Yet there was
a spell of a few weeks where every single week there was at least one
patch which was either applied twice, applied only partly or applied
to a wrong part of code. How can it "just work" if it so obviously
doesn't work?

All this means that a lot of effort of developers goes to a waste:
- because Bram can't (how could he possibly? He's a man: he needs to
eat, to sleep and to rest) evaluate all the possible patches and they
are added to the mythical todo.txt which means their changes of
getting included drop to almost 0,
- even though a patch got to todo.txt (and therefore will not be
included) people still work on it because they believe it will,
- people prepare a patch to fix something, a fix is published but the
solution has been totally rewritten; not a single word of explanation
is given,
- people prepare a patch, it gets included but for some reason the bug
doesn't go away. I recently had to re-debug Vim after my fix got
included only to discover that a line from my patch was not included.
The funny thing was that I did include a test and this single stable
branch of Vim was published with a failing test.

For some reason how people waste their time is OK because they volunteer
to waste their time, they are the guilty ones. But it also means that
Bram's time gets wasted:
- he needs to update todo.txt,
- he won't explain what was wrong with a patch so people will submit
those incorrect patches which Bram will have to rewrite,
- he needs to fix the problems with the mis-applied patches.

I used the pronoun "we" to mean "community" but the sorry state of Vim
development community is that IT DOES NOT EXIST. "Community" implies
a N-to-M relation between its members where N,M > 1. Here it's
a one-to-many relation between Bram and multiple patch submitters
because Bram has to evaluate every single patch. This means there's very
little incentive to examine each other's work (I will check your patch,
this way you will be more likely to check mine). That's why these
I-would-appreciate-it-if-some-people-tried-it-out emails from Bram get
so little response (and because people know that their work might go to
a waste). People work to scratch their own itches; there's no point in
scratching the itches of others.

I could write more but I think there's no point – this already got quite
long. I believe these things drive people away from Vim development or
make them one-time contributors. There used to be a nice small community
of contributors to vim-extended. We produced a number of stable, well
tested (we – as in more than an I – used them on daily basis!) features,
most of which got abandoned (because there was no change of them going
into mainline Vim. Instead the (still buggy) conceal feature got
included because someone convinced Bram it would be nice to be able turn
a text editor into e.g. a web browser. Some bugs simply got renamed as
features and everyone's happy.

I don't know why I'm writing this. I don't hope this email will change
anything. I *AM SORRY* and perhaps I believe that sharing will somehow
help me. I am a contributor of around 50-100 patches to Vim, I've been
here for 5 years and I don't feel I'm being taken seriously. I know
there are many others that will come and replace me, people whose work
could replace what I have done (I have seen one of the issues in
todo.txt solved 3 times before it got removed from todo.txt) but this
doesn't make it any less wrong. I don't deserve it. And I don't deserve
to have this email ignored.

Lech Lorens

[Yukihiro Nakadaira]

How about this?

I guess the key is ":setl scb | wincmd p", executing these two command
in one normal_cmd() session.

diff -r c015eedb9b4a src/testdir/test98.in
--- a/src/testdir/test98.in    Sun Jul 14 15:06:50 2013 +0200
+++ b/src/testdir/test98.in    Mon Jul 15 11:37:44 2013 +0900
@@ -1,7 +1,29 @@
 Test for 'scrollbind' causing an unexpected scroll of one of the windows.
 STARTTEST
 :so small.vim
-:source test98a.in
+:" We don't want the status line to cause problems:
+:set laststatus=0
+:let g:totalLines = &lines * 20
+:let middle = g:totalLines / 2
+:wincmd n
+:wincmd o
+:for i in range(1, g:totalLines)
+:    call setline(i, 'LINE ' . i)
+:endfor
+:exe string(middle)
+:normal zt
+:normal M
+:aboveleft vert new
+:for i in range(1, g:totalLines)
+:    call setline(i, 'line ' . i)
+:endfor
+:exe string(middle)
+:normal zt
+:normal M
+:" Execute the following two command at once to reproduce the problem.
+:setl scb | wincmd p
+:setl scb
+:wincmd w
 :let topLineLeft = line('w0')
 :wincmd p
 :let topLineRight = line('w0')
diff -r c015eedb9b4a src/testdir/test98a.in
--- a/src/testdir/test98a.in    Sun Jul 14 15:06:50 2013 +0200
+++ /dev/null    Thu Jan 01 00:00:00 1970 +0000
@@ -1,28 +0,0 @@
-" We don't want the status line to cause problems:
-set laststatus=0
-redraw!
-let g:totalLines = &lines * 20
-let middle = g:totalLines / 2
-wincmd n
-wincmd o
-for i in range(1, g:totalLines)
-    call setline(i, 'LINE ' . i)
-endfor
-
-exe string(middle)
-normal zt
-normal M
-
-aboveleft vert new
-for i in range(1, g:totalLines)
-    call setline(i, 'line ' . i)
-endfor
-exe string(middle)
-normal zt
-normal M
-setl scb
-
-wincmd p
-
-setl scb
-wincmd w

--
Yukihiro Nakadaira - yukihiro....@gmail.com

[Nikolay Pavlov]

Just in case somebody missed previous threads on the similar topic:
- https://groups.google.com/forum/m/#!topic/vim_dev/OlkOgDEDAq0
- https://groups.google.com/forum/m/#!topic/vim_dev/hAg_60fEdvM
- https://groups.google.com/forum/m/#!topic/vim_dev/78RMC7ajGfE

> --
> --
> You received this message from the "vim_dev" maillist.
> Do not top-post! Type your reply below the text you are replying to.
> For more information, visit http://www.vim.org/maillist.php
>
> ---
> You received this message because you are subscribed to the Google Groups "vim_dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+u...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[Tony Mechelynck]

I wouldn't say Vim development is utterly broken. Your rant above raises
some valid points, though others are exaggerated. The conceal feature
had been an unofficial patch since 6.2, maybe earlier, until finally
finally it got included in 7.3. Similarly the second half of the
floating point feature had been an unofficial patch, though not as long,
before it got included. In fact, Vim 7.3 was "enriched" with quite a
number of features which had existed for some time as "unofficial
patches", and been included /à la carte/ by a significant minority of
users. OTOH inclusion of the NFA engine, especially as "enabled by
default", seems to me to have been a hasty decision. This said, when you
say that "the development community does not exist", I think it is a
gross exaggeration. I see a lot more patches being proposed in vim_dev
nowadays than I used to see some ten years ago. Of course they aren't
all 100% perfect from day one, but that's the nature of bug fixing. And
yes, the bug fixing tends to be many-to-one rather than many-to-many but
sending bugs and proposed patches to the list (rather than by
confidential email) leaves the possibility open of having any patch
checked by someone other than Bram before it gets incorporated in the
code. Even so, the process is not strictly unicentric: Björn Winckler
(for the MacVim GUI) and Chip Campbell (for several runtime scripts) are
quite active (especially if you include vim_mac and vim_use in your
activity statistics) and there are "maintainers" for other parts of the
code (e.g. Nadim Shaikli for the Arabic module) who are less active
day-to-day but are still listed as maintainers.

Yes, I think Vim has grown, or will soon grow, to the point where at
least two pairs of eyes on every changes would be a useful development
feature: Here is what we have now (for C modules):

case 1:
- Someone reports a bug.
- Bram writes a patch.
- The patch gets included.

case 1a (variant of the above):
- Someone reports a bug.
- Bram adds it to the TODO list.
- A few years later, Bram writes a patch.
- The patch gets included.

case 2:
- Someone finds a bug, or decides to tackle a TODO list item, and writes
a patch, which he sends to vim_dev.
- Bram reviews the patch, possibly alters it a bit, but does not explain
what was wrong.
- The patch gets included.

The two-pairs-of eyes system has arguments for and against it. Here is a
possible procedure, as an alternative to the above. Vim "developers",
feel free to tear it to shreds:

1. Someone reports a bug and sends it to the vim_dev list.
-- Between steps 1 and 2 some time (years, even) may pass. I guess some
discussion might happen (is it a bug or a feature? Shall it go on the
TODO list? And how high on the list? Probably unclassified until a
consensus emerges.)
2. Someone (the same or another) writes a patch, and sends it to the
same thread of the list, asking (by name) someone known to be
knowledgeable about that part of the code to review it. (For code
written by someone else than Bram, the reviewer would most often be
Bram, but not necessarily. Code written by Bram would ideally still have
to be reviewed by someone else.)
3. The reviewer passes or fails the patch. An r- mark is always
accompanied by an explanation of what is wrong, an r+ may be accompanied
by a remark about something which should be corrected but doesn't need
re-review.
4. If r- the patch author makes the necessary changes (and fixes any
errors that he found himself by rereading or testing) and sends the
corrected patch back to the reviewer. Go to step 3. If r+ the patch
author fixes any remaining small nits and sends the patch to Bram (or to
someone with push permission on the master repo, but at the moment that
means Bram).
5. Bram has a right of "superreview". He doesn't have to spend time
checking every patch in full detail (he checks those where he's been
chosen as "reviewer", or as "additional reviewer" if the first reviewer
feels not fully competent, and any additional bugs which feels like
reviewing) but if he notices something wrong with a patch, it's back to
step 3, with explanations about what is wrong. Otherwise the patch gets
checked in.

This is only an outline. Some parts of the code (mostly in the runtime
files) already have "owners" (the so-called "maintainers"); similarly,
most of the C code is Bram's responsibility (but not all of it, see
above). It might be useful to find people whom Bram can trust (with
review) for some specific parts of the code; similarly, if some existing
maintainers find someone they can trust as "vice-maintainer" if
something happens to them, why not?

The advantage of this procedure is that several pairs of eyes are better
than one to catch bugs. But there are disadvantages:
a) It's slower than what we have now.
b) It floods the vim_dev list with discussions back and forth about each
and every bug, and that might be too much for which some users, even
some "technical" ones.


I think that (a) is inherent in any procedure where more than one pair
of eyes have to see every change; IMHO it's the flip side of trying to
catch more bugs before they get into the code.
About (b), I want to stress that the back-and-forth discussion is
necessary from a pedagogical point of view (people interested in fixing
bugs in some particular part of the code get to learn the ins and outs
of that part "at an elder's knee") and from a redundancy point of view
(everything is checked at least twice, and no change goes in
unreviewed); but I can imagine that it list flooding might become a
problem at some future point; we would then need a "real" bug tracker,
which doesn't send all bug-fixing discussions to the one list, but only
to the reviewer, the patch author (if any), and anyone having expressed
an interest (or been asked for advice) about the bug, or about the part
of the code the bug is in. Maybe (but it doesn't have to be implemented
immediately) something like Bugzilla, which is widely known and much
used (by 1268 companies as of 10 May 2012, according to
http://www.bugzilla.org/installation-list/ ). However I don't know how
heavy Bugzilla is in terms of infrastructure; but I think this is a
long-term evolution.


Best regards,
Tony.
--
If you've done six impossible things before breakfast, why not round it
off with dinner at Milliway's, the restaurant at the end of the universe?
-- Douglas Adams, "The Restaurant at the End of the Universe"

[ZyX]

I would vote against any development process that uses patches, be it current one, suggested two-eye system or anything else.

The two-eye system looks reasonable, but it should be applied to PR’s (commit series requested for inclusion as a whole) and not patches:


This is how we can protect from partial application of the patch and any possible problems with encoding, empty files or whatever which may happen on reviewers machines. Pulling is more predictable, mercurial is known to pull or, in rare cases, not to pull (e.g. when there are problems with plugins, enabled on server, but not on client or too old mercurial version on one side) without any possible states in between.

This is how we can protect from false commit messages (like the one where I was blamed for fixing some two problems in python tests while I actually said the opposite: these two problems are the only ones that are still not fixed). Patches do not have messages attached. Messages from Bram are also known to constantly include bits of information which seem not essential for me and omitting essential ones. It does not mean they are always inessential and Bram is completely wrong: e.g. pyeval() has proved to be more useful then I though when writing it, though vim.bindeval seemed more essential. But I have more details for this reason and they are always omitted.

Word about keeping context and seeing whether patch can be automatically merged was already said.

This also enables normal annotate and log: with current commit messages format almost every bit of useful data is stripped from annotate and log (without -v) output: no user, shortened commit message is useless as it says only about vim version. Problem/solution is too verbose, no summary in the first line. Most developers will write this in their messages thus these features will work regardless of what is written in the merge message.

This is also how one can easily see which parts were included by Bram unmodified and which ones were modified (e.g. to fit coding style). This makes me learn about what should I be aware of when writing patches.


I cannot say what are the advantages for Bram regarding simplicity of the process without knowing how he does his job, but with PR’s at least problems with omitting parts of the patches and noise about wrong commit messages will go away. In addition to noise about not using DVCS as DVCS and broken development process. I cannot think of any sanely written script that may modify my patch to leave one parenthesis as-is (like it was done recently), thus I assume 1. job of applying patches is done manually and 2. manually taking patches out of the messages is hard enough to apply smaller ones by hand. Do not know about bitbucket (did not ever received PR’s here), but with github (assuming Bram does not want to switch context from mailer to browser) merging basically is “copy command from email message to shell and run it”. Reviewing the diff is “open URL (under ‘Patch links’ section) in Vim and review” (since vim is capable for opening https:// links). I hope bitbucket has something like this.

[Ben Fritz]

On Monday, July 15, 2013 9:31:00 AM UTC-5, ZyX wrote:
> I would vote against any development process that uses patches, be it current one, suggested two-eye system or anything else.
>
>
>
> The two-eye system looks reasonable, but it should be applied to PR’s (commit series requested for inclusion as a whole) and not patches:
>
>
>
>
>
> This is how we can protect from partial application of the patch and any possible problems with encoding, empty files or whatever which may happen on reviewers machines.

Since we're using Mercurial, and Bram wants to allow email submissions, a developer can email a "bundle" if he/she doesn't have/want access to a public repository. Then just the commits required for the fix (with some commit also in Bram's repository as a parent) need to be emailed, and he can import or pull from the bundle.

I'm not altogether opposed to using patches for an initial solution, but at some point the patch needs to go into a real commit so people can review it BEFORE it gets released officially.

I think using rebase risks some of the same problems that patches have, so a completely linear history may be harder to keep. But, Bram can at least keep using a linear set of tagged revisions with nothing in between, by updating the version.c version number and applying a tag after every merge he intends to publish.

[George V. Reilly]

(It's been a while since I've been an active contributor, but I still
lurk here and read some of the threads.)

Using a proper DVCS workflow is far superior to having one BDFL manually
apply patches. That was the best we could do in the 1990s; we have
better choices now. For more than a year at the day job, we've been
using Github Pull Requests to manage how features are merged onto the
master branch of our various repositories. We get an easy way of code
reviewing, automated test results from our continuous integration
server, an audit trail, and a rarely-needed mechanism for reverting.
Between our various repos, our team of 15 engineers has opened (and
closed) almost 1,000 pull requests. It's a successful workflow.

Bitbucket also has Pull Requests [1], though I haven't worked with them.
Google Code does not [2].

1: https://confluence.atlassian.com/display/BITBUCKET/Work+with+pull+requests
2: https://code.google.com/p/support/issues/detail?id=4753
--
/George V. Reilly geo...@reilly.org Twitter: @georgevreilly
http://www.georgevreilly.com/blog http://blogs.cozi.com/tech

[ZyX]

> Since we're using Mercurial, and Bram wants to allow email submissions, a developer can email a "bundle" if he/she doesn't have/want access to a public repository. Then just the commits required for the fix (with some commit also in Bram's repository as a parent) need to be emailed, and he can import or pull from the bundle.

Bundles solve exactly one problem: they avoid problems with applying patches. They do introduce another problem: now it is tricky to see the diff (any discussion around PR’s assumes having web interface for them). With bundles one still needs to repost each time he has an update (common for emailed bundles and diff).

> I'm not altogether opposed to using patches for an initial solution, but at some point the patch needs to go into a real commit so people can review it BEFORE it gets released officially.
>
> I think using rebase risks some of the same problems that patches have, so a completely linear history may be harder to keep. But, Bram can at least keep using a linear set of tagged revisions with nothing in between, by updating the version.c version number and applying a tag after every merge he intends to publish.

It is part of what (a successful) git-flow model suggests. As far as I see git-flow suits perfectly for mercurial as well.

Note that rebasing of a commit from third-party source is tricky because mercurial uses phases for preventing you from doing such mistake.

[glts]

I recently had an encounter with the dev process used for Go. They use
Mercurial with the Rietveld code review extension. Every change must be
submitted for public review first and needs explicit approval from the
devs/community before receiving an LGTM ("looks good to me") and commit.

I found it pretty impressive, might be worth a look?
https://groups.google.com/forum/#!forum/golang-dev
https://codereview.appspot.com/

Best,

[Ben Fritz]

On Monday, July 15, 2013 11:33:55 AM UTC-5, ZyX wrote:
> > Since we're using Mercurial, and Bram wants to allow email submissions, a developer can email a "bundle" if he/she doesn't have/want access to a public repository. Then just the commits required for the fix (with some commit also in Bram's repository as a parent) need to be emailed, and he can import or pull from the bundle.
>
>
>
> Bundles solve exactly one problem: they avoid problems with applying patches. They do introduce another problem: now it is tricky to see the diff (any discussion around PR’s assumes having web interface for them).

No, the discussion only assumes that everyone can see the same changes. It's easy enough to get the changes into a personal clone on your system to view them. That is an early step in merging and applying them anyway. I assume any changes merged in from pull requests must be tested before publishing.

> With bundles one still needs to repost each time he has an update (common for emailed bundles and diff).
>

I view that as a good thing. You'll need to post anyway when you make an update, otherwise nobody knows about it unless they are following your repository very closely. If Bram has 12 pull requests, he probably won't want to monitor each repository to see if anything changes. So the pull requester must post to say "updated to fix XYZ". If the changes are immediately available in the email, the maintainer has less work left to do to get them.

That said, I'd probably prefer to just pull from a public repository myself, if I knew the correct revision to pull in.

[ZyX]

> > Bundles solve exactly one problem: they avoid problems with applying patches. They do introduce another problem: now it is tricky to see the diff (any discussion around PR’s assumes having web interface for them).
>

> No, the discussion only assumes that everyone can see the same changes. It's easy enough to get the changes into a personal clone on your system to view them. That is an early step in merging and applying them anyway. I assume any changes merged in from pull requests must be tested before publishing.

Before pulling into my clone I normally view diff to check out that it is worth pulling at all. Some problems do not need pulling to be discovered.

> > With bundles one still needs to repost each time he has an update (common for emailed bundles and diff).
>
> I view that as a good thing. You'll need to post anyway when you make an update, otherwise nobody knows about it unless they are following your repository very closely. If Bram has 12 pull requests, he probably won't want to monitor each repository to see if anything changes. So the pull requester must post to say "updated to fix XYZ". If the changes are immediately available in the email, the maintainer has less work left to do to get them.

?! You don’t need to post, you need to push to a proper branch and it will appear in PR automatically. AFAIR github does not bother notifying in this case, dunno about bitbucket, but it would be good thing to do (I mean, notify about push into PR).

> That said, I'd probably prefer to just pull from a public repository myself, if I knew the correct revision to pull in.

Github mails you with a correct command to pull. Anybody here has an experience with bitbucket regarding pull requests?

[Marc Weber]

> of contributors to vim-extended.
Link? I'd like to add it to
http://vim-wiki.mawercer.de/wiki/vim-development/recent-work.html

If you come up with something know consider adding it to:
http://vim-wiki.mawercer.de/wiki/vim-development/development.html

Should we start fund raising for development? Would this improve
anything?

Marc Weber

[ZyX]

> Is heavily working on improving the Python API. His description about the changes: https://code.google.com/p/vim/

?! Where is my description on the main project page?

[Marc Weber]

> ?! Where is my description on the main project page?

Sry, fixed to: https://gist.github.com/ZyX-I/5561409

[Ben Fritz]

On Monday, July 15, 2013 12:27:03 PM UTC-5, ZyX wrote:
> > > With bundles one still needs to repost each time he has an update (common for emailed bundles and diff).
> >
> > I view that as a good thing. You'll need to post anyway when you make an update, otherwise nobody knows about it unless they are following your repository very closely. If Bram has 12 pull requests, he probably won't want to monitor each repository to see if anything changes. So the pull requester must post to say "updated to fix XYZ". If the changes are immediately available in the email, the maintainer has less work left to do to get them.
>
> ?! You don’t need to post, you need to push to a proper branch and it will appear in PR automatically. AFAIR github does not bother notifying in this case, dunno about bitbucket, but it would be good thing to do (I mean, notify about push into PR).
>

I have so far not been active enough in any open-source project to use the web-based tools for pull requests, so I admit ignorance in regard to how that works. I have so far assumed it is mostly a streamlined way (with a nice interface) to do "please pull revision abc123 from my public repository at example.com/repo to get my changes for feature X". I.e. this is something that COULD be done manually but is more efficient with the GitHub/BitBucket interface.

I may have overlooked the idea that the maintainer should be looking at their own repository frequently enough to see that there is a pending pull request.

Nevertheless, I do NOT want to pull a PR which I assume has been reviewed completely, only to have a few recently-pushed changes "sneak in" without review. I do NOT want to monitor for updates to the pull request to know when a fix I request is complete, I want to be told "hey, I fixed that issue you found in review, try again". I do NOT want to do the pull request, then go to publish the result, only to discover that some last-minute changes were made after my pull and need to pull again.

Maybe it is just my ignorance showing; maybe the GitHub/BitBucket pull request concept is good enough to handle all that, but it seems like a good way to accomplish these things is a simple email to the list or maintainer saying "I updated pull request with revision abc123def to fix Jane Doe's review findings, it should be ready again".

And not everybody has or wants a GitHub account. I think email submissions are still good, for small changes at the very least.

My point was that it should be just as easy to say "I updated to fix Jane Doe's review findings, see attached bundle". Or even "see attached patch based off version 123fe56 in the main repo". With the bundle there is less opportunity for error than the patch.

[Nikolay Pavlov]

On Jul 15, 2013 11:22 PM, "Ben Fritz" <fritzo...@gmail.com> wrote:
>
> On Monday, July 15, 2013 12:27:03 PM UTC-5, ZyX wrote:
> > > > With bundles one still needs to repost each time he has an update (common for emailed bundles and diff).
> > >
> > > I view that as a good thing. You'll need to post anyway when you make an update, otherwise nobody knows about it unless they are following your repository very closely. If Bram has 12 pull requests, he probably won't want to monitor each repository to see if anything changes. So the pull requester must post to say "updated to fix XYZ". If the changes are immediately available in the email, the maintainer has less work left to do to get them.
> >
> > ?! You don’t need to post, you need to push to a proper branch and it will appear in PR automatically. AFAIR github does not bother notifying in this case, dunno about bitbucket, but it would be good thing to do (I mean, notify about push into PR).
> >
>
> I have so far not been active enough in any open-source project to use the web-based tools for pull requests, so I admit ignorance in regard to how that works. I have so far assumed it is mostly a streamlined way (with a nice interface) to do "please pull revision abc123 from my public repository at example.com/repo to get my changes for feature X". I.e. this is something that COULD be done manually but is more efficient with the GitHub/BitBucket interface.
>
> I may have overlooked the idea that the maintainer should be looking at their own repository frequently enough to see that there is a pending pull request.
>
> Nevertheless, I do NOT want to pull a PR which I assume has been reviewed completely, only to have a few recently-pushed changes "sneak in" without review. I do NOT want to monitor for updates to the pull request to know when a fix I request is complete, I want to be told "hey, I fixed that issue you found in review, try again". I do NOT want to do the pull request, then go to publish the result, only to discover that some last-minute changes were made after my pull and need to pull again.

Normally on both sides are reasonable people. One does not say PR is finished and add a new commit after this without notification unless he knows for sure when review is done. Though there also is a problem in github: I have same reasons to expect it to notify about changes, but it does not. Hope bitbucket does, but this needs to be tested.

> Maybe it is just my ignorance showing; maybe the GitHub/BitBucket pull request concept is good enough to handle all that, but it seems like a good way to accomplish these things is a simple email to the list or maintainer saying "I updated pull request with revision abc123def to fix Jane Doe's review findings, it should be ready again".

I did the same by posting to PR if I expected other side to miss changes (normally always after review had happened due to lack of notification). There now seems to be no advantage with github.

Bitbucket has approve button in PR, but pushing new changeset does not alter approval status. It also does not notify about any changes as I run PR from one my repository to the other and as I approve myself and also push changes by myself the actual problem may be that it tries to be smart and thus changes pushed by approver just do not alter approval status. Second person is needed for the test. It did not automatically add changes to PR in my simple test though.

Github does not have support for approving PRs.

> And not everybody has or wants a GitHub account. I think email submissions are still good, for small changes at the very least.

I actually do not want it on github, I would want it on bitbucket assuming it has comparable features. But as it being not so popular I have no experience with collaborative development there and cannot evaluate pros and cons for sure. Issue tracker is much better on bitbucket.

> My point was that it should be just as easy to say "I updated to fix Jane Doe's review findings, see attached bundle". Or even "see attached patch based off version 123fe56 in the main repo". With the bundle there is less opportunity for error than the patch.
>

[Bram Moolenaar]

Yukihiro Nakadaira wrote:

> How about this?
>
> I guess the key is ":setl scb | wincmd p", executing these two command
> in one normal_cmd() session.

Wasn't there a reason these commands were in a separate file?

As mentioned before, we should have a test for the tests...


--
Back off man, I'm a scientist.
-- Peter, Ghostbusters

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[Yukihiro Nakadaira]

On Tue, Jul 16, 2013 at 5:17 AM, Bram Moolenaar <Br...@moolenaar.net> wrote:

Yukihiro Nakadaira wrote:

> How about this?
>
> I guess the key is ":setl scb | wincmd p", executing these two command
> in one normal_cmd() session.

Wasn't there a reason these commands were in a separate file?

As mentioned before, we should have a test for the tests...

I think that the separate file was used for same purpose.

[Bram Moolenaar]

Sorry, I don't understand what you mean with "same purpose".

--
For large projects, Team Leaders use sophisticated project management software
to keep track of who's doing what. The software collects the lies and guesses
of the project team and organizes them in to instantly outdated charts that
are too boring to look at closely. This is called "planning".
(Scott Adams - The Dilbert principle)

[Tony Mechelynck]

On 07/15/13 16:31, ZyX wrote:
> I would vote against any development process that uses patches, be it current one, suggested two-eye system or anything else.
>
> The two-eye system looks reasonable, but it should be applied to PR’s (commit series requested for inclusion as a whole) and not patches:

By "a patch" I mean what can serve as input to one run of the "patch"
program, or what can become one new Mercurial changeset. In my mind, one
"patch" can include several (related) changes to different files, meant
to be pushed together. See for example most patch-type attachments to
bug reports at bugzilla.mozilla.org. Every patchlevel of Vim
(corresponding currently to one line each after line 28 of
ftp://ftp.vim.org/pub/vim/unstable/patches/7.4a/README ) is what I call
"one patch".
This is not like what you can do online at github, where it is not
possible to modify several files in one commit, except by doing it in a
clone on your HD then pushing over SSL (requiring a GPG passphrase, and
I've forgotten mine), and where future bugfixes are committed as real
changesets into "side branches" ("branches" in the git sense) of an
online clone. At Mozilla, each patch (a diff of one or more files as
kept in .hg/patch* by the mq extension) gets attached as a unified diff
in git format to the concerned bug. A later version of a patch (in diff
format) may "obsolete" a previous one; then when the final version of a
patch has received the necessary reviews (and possibly superreview,
approval, etc.) the patch author (who is often not a member of the
select group of people which push access to the Mercurial repo) sets the
"checkin-needed" keyword on the bug, and someone else, maybe a Mozilla
employee or volunteer, but with push access, who periodically runs a
search for open bugs having that keyword, will import that patch into
his own clone (using the mq extension), check that it applies cleanly,
that it has the necessary metadata (e.g. an author name, a Subject
starting with a bug number, the names or IRC nicks of who gave the r+
(review) and maybe ui-r+ (UI review), sr+ (superreview) and/or a+
(approval) checks, etc. Then if the patch is OK he will commit it, for
SeaMonkey or Thunderbird to the comm-central repo, for Firefox often to
the mozilla-inbound repo, which gets merged into the mozilla-central
repo maybe once a day or so, when "the tree is green", i.e., the code
currently committed not only compiles and links, but passes all
automatic tests on all platforms (Win32, Lin32, Lin64 and Mac-Universal).

>
>
> This is how we can protect from partial application of the patch and any possible problems with encoding, empty files or whatever which may happen on reviewers machines. Pulling is more predictable, mercurial is known to pull or, in rare cases, not to pull (e.g. when there are problems with plugins, enabled on server, but not on client or too old mercurial version on one side) without any possible states in between.

To address the possibility of bit-rot, the patch author should refresh
his clone before every change, as well as when requesting
checkin-needed. Use of the mq extension to Mercurial is IMHO strongly
recommended, so that a patch can be developed, compiled, tested, and yet
popped off the clone when desired (e.g. while pulling updates from the
master repo).

I also use the mq extension to import patches written by others which I
want to test, so that no patch pollutes my clone's history before I've
decided that I like it enough to commit it onto my unnamed side branch
(in the Mercurial sense, of course; a "branch" in the git sense would be
a "bookmark" on Mercurial) of local changes (as I did for "extended"
floating-point behaviour before it finally became part of Bram's Vim).
At the moment my only local changes are a few additions to .hgignore
(for instance my shadow directories, my cscope.out, and anything ending
in .log) and a couple of changes to src/feature.h (in order to compile
with -tag_old_static and +xterm_save, as there is no configure setting
about these).

>
> This is how we can protect from false commit messages (like the one where I was blamed for fixing some two problems in python tests while I actually said the opposite: these two problems are the only ones that are still not fixed). Patches do not have messages attached. Messages from Bram are also known to constantly include bits of information which seem not essential for me and omitting essential ones. It does not mean they are always inessential and Bram is completely wrong: e.g. pyeval() has proved to be more useful then I though when writing it, though vim.bindeval seemed more essential. But I have more details for this reason and they are always omitted.

Since the mq extension places the commit message at the top of the patch
(in what the manual for the patch program calls the "leading garbage"),
the text of the message can be checked by eyeballing the patch text, and
if necessary corrected by hg qrefresh -m 'new text' before the patch is
committed into the repo, and frozen in bronze as one more changeset.

>
> Word about keeping context and seeing whether patch can be automatically merged was already said.
>
> This also enables normal annotate and log: with current commit messages format almost every bit of useful data is stripped from annotate and log (without -v) output: no user, shortened commit message is useless as it says only about vim version. Problem/solution is too verbose, no summary in the first line. Most developers will write this in their messages thus these features will work regardless of what is written in the merge message.

I wouldn't know; I have verbose=true in the [ui] section of my ~/.hgrc.
I have also set "log" and "annotate" (among other hg commands) to
default to being paged (using the pager extension) and BTW my default
pager for hg output is 'view -'. ;-)

>
> This is also how one can easily see which parts were included by Bram unmodified and which ones were modified (e.g. to fit coding style). This makes me learn about what should I be aware of when writing patches.
>
>
> I cannot say what are the advantages for Bram regarding simplicity of the process without knowing how he does his job, but with PR’s at least problems with omitting parts of the patches and noise about wrong commit messages will go away. In addition to noise about not using DVCS as DVCS and broken development process. I cannot think of any sanely written script that may modify my patch to leave one parenthesis as-is (like it was done recently), thus I assume 1. job of applying patches is done manually and 2. manually taking patches out of the messages is hard enough to apply smaller ones by hand. Do not know about bitbucket (did not ever received PR’s here), but with github (assuming Bram does not want to switch context from mailer to browser) merging basically is “copy command from email message to shell and run it”. Reviewing the diff is “open URL (under ‘Patch links’ section) in Vim and review” (since vim is capable for opening https:// links). I hope bitbucket ha
s something like this.
>

Best regards,
Tony.
--
BLACK KNIGHT: I'm invincible!
ARTHUR: You're a looney.
"Monty Python and the Holy Grail" PYTHON (MONTY)
PICTURES LTD

[Marc Weber]

> checkin-needed. Use of the mq extension to Mercurial is IMHO strongly
> recommended, so that a patch can be developed, compiled, tested, and yet
> popped off the clone when desired (e.g. while pulling updates from the
> master repo).

http://vim-wiki.mawercer.de/wiki/vim-development/development.html
-> "keeping your own work up to date" talks about topgit and
PatchBranchExtension (hg) which are alternatives to mq.

They allow exporting a final commit based on the history of a topic
branch into which master/head got merged multiple times.

Well - at least its my preferred way :)

Marc Weber

[Yukihiro Nakadaira]

On Wed, Jul 17, 2013 at 5:29 AM, Bram Moolenaar <Br...@moolenaar.net> wrote:

Yukihiro Nakadaira wrote:

> On Tue, Jul 16, 2013 at 5:17 AM, Bram Moolenaar <Br...@moolenaar.net> wrote:
>
> >
> > Yukihiro Nakadaira wrote:
> >
> > > How about this?
> > >
> > > I guess the key is ":setl scb | wincmd p", executing these two command
> > > in one normal_cmd() session.
> >
> > Wasn't there a reason these commands were in a separate file?
> >
> > As mentioned before, we should have a test for the tests...
>
> I think that the separate file was used for same purpose.

Sorry, I don't understand what you mean with "same purpose".

To execute ":setl scb" and ":wincmd p" in one normal_cmd() session.

It can be :source, :function, or bar.

[Tony Mechelynck]

I guess you came to Mercurial from git (correct me if I'm wrong): this
"merged multiple times" business sounds like what happens at github. Me,
on the contrary, I've come to master Mercurial fairly well, but git,
when I have to use it (e.g. for projects whose source is held at github)
always seems foreign to me. With mq, you qpush a diff-format patch (on
top of the current changeset, usually the tip) when you're working on
it, you qrefresh and qpop it when you temporarily aren't, and anything
that you pull -u from the master repo in the meantime doesn't need to be
"merged" with it: the next time you qpush, the patch will be pushed onto
the new tip (and I'm not sure what happens in case of bit-rot since I've
never had the case).

Well, /de gustibus and coloribus non est disputandum/ — one shouldn't
quarrel about differences in taste. In Vim also, different people will
use different commands, or sequences of them, to achieve a given result.


Best regards,
Tony.
--
"If I believed in a god, which I do not, I would like to
communicate with him on the same intellectual level.
Therefore, I would have to teach him a few things."
[Aaron Erwin]

[Marc Weber]

You can share "topic branches" or "PatchBranchExtension". mq patches are
"your personal stack of patches" only AFAIK.

mq allows to reorder patches, pop/push them.
PatchBranchExtension is about managing patches which may depend on each
other and sharing them, right?

I've added this to the wiki.

> [..]

> use different commands, or sequences of them, to achieve a given result.

Yes - but there is :h to document and talk about all of them :) So its
not bad doing the same for Vim development workflows.

Marc Weber

[Tony Mechelynck]

On 07/17/13 00:59, Marc Weber wrote:
> You can share "topic branches" or "PatchBranchExtension". mq patches are
> "your personal stack of patches" only AFAIK.

You can share them in diff format, just like the patches that I see on
vim_dev.

>
> mq allows to reorder patches, pop/push them.
> PatchBranchExtension is about managing patches which may depend on each
> other and sharing them, right?

I don't know PatchBranchExtension. In mq, when patches depend on each
other, you keep them in one "queue" which is pushed and popped like a
stack. When they don't, you can put them in separate queues (with the hg
qqueue command) and push/pop them independently of each other. So far,
the Mozilla bugs I've fixed didn't require much coding (maybe a handful
of lines in up to three files) so I never felt the need to divide the
fixes for a single bug into several interdependent patches: typically my
"queues" are of one patch each; you're making me remember that I can
delete any of them which has made it into the official source. But I've
seen bigger fixes written by others, with (rarely) up to 8
interdependent patches that had to be pushed in a certain order.

>
> I've added this to the wiki.

URL?

>
>> [..]
>> use different commands, or sequences of them, to achieve a given result.
> Yes - but there is :h to document and talk about all of them :) So its
> not bad doing the same for Vim development workflows.

Yes indeed: Vim is the only piece of software with a help worth its name
that I've met since (when? in the eighties, maybe) I stopped working on
mainframes where the documentation (for software that came with the
machine) was a set of paper books (in quarto or US letter format, I
think, and meant to be kept in three-ring binders) and where the source
(in assembly language on half-inch 2400 ft tapes, and sometimes on
assembly listings on zigzag paper 132 characters wide by 11 or 12
inches, similar to "ledger" from what I see under :help popt-option) was
there for anyone interested in it — most people weren't.

I remember getting an interim job once after going to an "examination"
where we had to write a COBOL program about a given problem. I went with
the COBOL manual for my former computer, opened it in front of me on the
table, and stuck to the "standard" (non-machine-specific) language as
described in it. Some other examinees rebuked me for “cheating”, but I
got the job. (“Cheating?” I said, “Aren't you /supposed/ to have the
compiler manual at hand when writing a program?”).

>
> Marc Weber
>
Best regards,
Tony.
--
The UNIX philosophy basically involves giving you enough rope to
hang yourself. And then a couple of feet more, just to be sure.

[Marc Weber]

Excerpts from Tony Mechelynck's message of Wed Jul 17 03:45:35 +0200 2013:

> You can share them in diff format, just like the patches that I see on
> vim_dev.

I'm talking about "hg pull" like sharing.

> URL?
still the same: http://vim-wiki.mawercer.de/wiki/vim-development/development.html


The script below shows how you can have a topic branch, merge "default"
into it by "hg pmerge", and still pull/push it. Try the same with mq,
you'll end up in a mess (with distributed development)

That's the main difference. Person B can fix bugs in the topic branch of
Person A. Person C can develop a feature on top of that topic branch
etc. Whether this is useful depneds on the use case - but you can push
to bitbucket or the like.

Marc Weber

rm -fr test
(
set -x
set -e
mkdir -p test/source

add_commit(){
echo $1 > $1
hg add $1
hg commit -m $1
}

(
cd test/source
hg init .
add_commit a
hg pnew -t topic-msg topic

# this will not show up, because its committed upstream below, see [up]
add_commit topic-commit-1
# these will end up in the topic diff
add_commit topic-commit-2
)

# clone topic
hg clone test/source test/clone


(
cd test/source
hg update -C default
add_commit upstream-change

# let's make it more complicated, feed topic-commit upstream [up]
add_commit topic-commit-1


# add a third topic change:
hg update -C topic
add_commit topic-commit-3
)

(
cd test/clone
# now get updates of the topic branch by pull
# and this is what I don't like, you cannot "preview" using mercurial ..
# best you could do is make a backup of this repo before pulling ..
# so this should get topic-commit-3 into the clone
hg pull -u

hg update -C default
add_commit last
hg update -C topic
# merge this patch upstream
hg pmerge

# and this is the cool thing, you can just push the updated topic without
# "diff" formats:
hg push -b topic ../source
)


(
# export (we don't have deps, but this would export topics on which this topic depneds, too)
cd test/source
hg update -C topic
hg pexport -U 10
)

)

[Ben Fritz]

On Tuesday, July 16, 2013 9:16:09 PM UTC-5, MarcWeber wrote:
> Excerpts from Tony Mechelynck's message of Wed Jul 17 03:45:35 +0200 2013:
>
> > You can share them in diff format, just like the patches that I see on
>
> > vim_dev.
>
> I'm talking about "hg pull" like sharing.
>
>
>
> > URL?
>
> still the same: http://vim-wiki.mawercer.de/wiki/vim-development/development.html
>

Can you elaborate a little on the "remotes" feature you list on that page as being better in git?

IIUC I *think* that is basically the same as the "paths" section in the hgrc, but with an interface to modify it.

http://stackoverflow.com/questions/4956346/how-can-i-add-remote-repositories-in-mercurial

TortoiseHg provides a GUI to add paths, but I don't know of any built-in command-line tools to do it.