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
--
--
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 [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
