Re: [RFC PATCH 0/3] fast-import: add option command

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sverre Rabbelier <srabbelier@xxxxxxxxx> wrote:
>     This allows the frontend to specify any of the supported options as
>     long as no non-option command has been given. This way the
>     user does not have to include any frontend-specific options, but
>     instead she can rely on the frontend to tell fast-import what it
>     needs.
...
>  fast-import.c          |  137 ++++++++++++++++++++++++++++++++++++-----------
>  t/t9300-fast-import.sh |   33 ++++++++++++
>  2 files changed, 138 insertions(+), 32 deletions(-)

Some comments:

Since you are changing the language format, please also update the
documentation of the language.

It might be cleaner to say "option foo=value\n" because then the
if block to parse the command line and the if block to parse the
input stream are the same.  When parsing argv just skip the "--"
and pass the rest of the argv value into the function, when parsing
the stream, just skip the "option " and pass the rest of the line
into the function.

This has come up before on the list.  We had somewhat decided against
setting options in the stream header.  The only option class that
really impacts the data itself is the date format, all others
are about local file paths or how noisy the command should be.
Thus we decided that the frontend should invoke `git fast-import`
itself if it cared about these options, and that's what any typical
frontend does.

-- 
Shawn.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]