From: "Shawn O. Pearce" <spearce@spearce.org>
To: Sverre Rabbelier <srabbelier@gmail.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
Git List <git@vger.kernel.org>,
Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 2/4] fast-import: define a new option command
Date: Thu, 13 Aug 2009 15:12:13 -0700 [thread overview]
Message-ID: <20090813221213.GR1033@spearce.org> (raw)
In-Reply-To: <fabb9a1e0908131501o73807f81mb230530c284ad123@mail.gmail.com>
Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Thu, Aug 13, 2009 at 14:51, Johannes
> Schindelin<Johannes.Schindelin@gmx.de> wrote:
> > Sorry if I spoil the party, but maybe if things get so complicated, it may
> > be a sign that the original version (stream overrides command line, since
> > it knows better) is to be preferred? ?After all, if hg fast-export says
> > that the marks should be imported from a certain file, it may be for a
> > _very good_ reason...
>
> Yes, and that should Just Work (which it does). Also, I'm not sure how
> often one would output a stream on one computer, then move it to
> another and import it there, but I'll methinks Shawn brought it up for
> a reason ;). However, I do think it's better design to only store the
> name of the import file and then do the actual import later on (to
> prevent double imports).
>
> I don't have a preference either way (both patches are already written
> after all). Shawn?
No, I don't have a really good reason for the command line overrides
the file other than this simple rule:
If the file is likely to be several hundred MiB, or bigger; thou
shall never try to open it with vi, *especially* vi on a Solaris
system, as at least one line is likely to be too long.
If the file header contains paths to other files, it is likely
one will want to modify that header sometime, because you moved
the file between systems.
Given the size of the file above, you can't just fix it with vi.
Lacking a tool that *can* do this edit safely (and Dscho's simple
sed wasn't enough, as I already pointed out, oh and Solaris sed
also fails on long lines), we *should* be able to override this on
the command line, *especially* since we already have the command
line option standardized!
What is this, gang up on Shawn's words-of-wisdom week? Both this
thread and my intern this week have been argueing with me about
what seem to me to be fairly trivial things. Maybe I just need to
take vacation. Good thing I have one coming in 5 weeks.
I say use the version where we store the values (e.g. file names)
during option parsing, and then actually apply those saved values
just before the first non-option command. Which I think only has
an impact on the import-marks option, the rest are all just simple
variable updates whose values aren't read until after the first
non-option command anyway.
--
Shawn.
next prev parent reply other threads:[~2009-08-13 22:12 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-08-13 5:09 [PATCH 0/4] fast-import: add a new option command Sverre Rabbelier
2009-08-13 5:09 ` [PATCH 1/4] fast-import: put option parsing code in seperate functions Sverre Rabbelier
2009-08-13 5:09 ` [PATCH 2/4] fast-import: define a new option command Sverre Rabbelier
2009-08-13 5:09 ` [PATCH 3/4] fast-import: add " Sverre Rabbelier
2009-08-13 5:09 ` [PATCH 4/4] fast-import: test the new " Sverre Rabbelier
2009-08-13 14:45 ` [PATCH 3/4] fast-import: add " Shawn O. Pearce
2009-08-13 14:43 ` [PATCH 2/4] fast-import: define a new " Shawn O. Pearce
2009-08-13 14:56 ` Johannes Schindelin
2009-08-13 15:04 ` Shawn O. Pearce
[not found] ` <fabb9a1e0908130812s297ccfc6vd6b746daf1dcc69a@mail.gmail.com>
2009-08-13 15:24 ` Shawn O. Pearce
2009-08-13 16:26 ` Sverre Rabbelier
2009-08-13 17:07 ` Johannes Schindelin
2009-08-13 17:09 ` Sverre Rabbelier
2009-08-13 17:25 ` Shawn O. Pearce
2009-08-13 17:28 ` Sverre Rabbelier
2009-08-13 17:41 ` Shawn O. Pearce
2009-08-13 17:44 ` Sverre Rabbelier
2009-08-13 17:52 ` Shawn O. Pearce
2009-08-13 21:51 ` Johannes Schindelin
2009-08-13 22:01 ` Sverre Rabbelier
2009-08-13 22:12 ` Shawn O. Pearce [this message]
2009-08-13 22:17 ` Sverre Rabbelier
2009-08-13 19:26 ` Junio C Hamano
2009-08-13 20:01 ` Sverre Rabbelier
2009-08-13 20:42 ` Junio C Hamano
2009-08-13 21:14 ` Sverre Rabbelier
2009-11-23 17:39 ` Sverre Rabbelier
2009-11-23 20:50 ` Shawn O. Pearce
2009-08-13 10:19 ` [PATCH 1/4] fast-import: put option parsing code in seperate functions Johannes Schindelin
2009-08-13 14:40 ` Shawn O. Pearce
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090813221213.GR1033@spearce.org \
--to=spearce@spearce.org \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=srabbelier@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox