Re: [PATCH] cvsimport: new -R option: generate revision-to-commit mapping

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

 



On Sun, Jan 31, 2010 at 12:43:44PM +0000, Aaron Crane wrote:

> Signed-off-by: Aaron Crane <git@xxxxxxxxxxxxxxxx>
> ---

Please put a bit of the rationale into the commit message. Even a
sentence or two can help later on when somebody is reading the output of
"git log".

>  Documentation/git-cvsimport.txt |   15 ++++++++++++++-
>  git-cvsimport.perl              |   20 ++++++++++++++++----
>  2 files changed, 30 insertions(+), 5 deletions(-)

A basic test would be nice. You should be able to just use your new "-R"
during the import in t9600, and then check that it generated the correct
mapping.

> +-R <revision-to-commit-file>::
> +	Generate a file containing a mapping from CVS revision numbers to
> +	newly-created Git commit IDs.  The generated file will contain one
> +	line for each (filename, revision) pair found by 'cvsps'; each line
> +	will look like

Is mentioning 'cvsps' right here?  cvsps doesn't know about git commit
id's.

> +open my $revision_map, '>', $opt_R
> +    or die "Can't open -R file $opt_R: $!\n"
> +	if defined $opt_R;

You need to use munge_user_filename here to handle relative paths. See
commit f6fdbb6.

Also, should you perhaps be appending to the file instead of truncating
it? Remember that cvsimport can be used incrementally. I wonder if it
would be better to simply have "-R" without an argument to append the
revision map to a file .git/cvs-revisions or something. And then the
user can easily pull it from there after the import.

-Peff
--
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]