From: "David D. Kilzer" <ddkilzer@kilzer.net>
To: "Eric Wong" <normalperson@yhbt.net>
Cc: git@vger.kernel.org, "David D. Kilzer" <ddkilzer@kilzer.net>
Subject: [PATCH 0/3] Implement git-svn: info
Date: Tue, 20 Nov 2007 22:43:16 -0800 [thread overview]
Message-ID: <1195627399-25209-1-git-send-email-ddkilzer@kilzer.net> (raw)
In-Reply-To: <20071117225402.GC28755@muzzle>
I've split the original RFC patch into two patches (one for the
refactoring changes, and one for implementing the "git svn info"
command), and added a third to imlement "git svn info --url" per
Eric's request.
Eric Wong <normalperson@yhbt.net> wrote:
> "David D. Kilzer" <ddkilzer@kilzer.net> wrote:
> > Implement "git-svn info" for files and directories based on the "svn info"
> > command. Note that the -r/--revision argument is not supported yet.
> [...]
> Wow. I can honestly say I've never even noticed the "Schedule:" field
> in `svn info'. I would've been perfectly happy to accept an
> implementation of `git svn info' without that :)
I never noticed it, either, until I went to implement this feature!
My goal is to stay as true to the "svn info" output as "git svn log"
has done for "svn log".
> > I've also tried to be aggressive in extracting common code into functions.
> I like it, but this should be a separate patch.
See Patch 1/3.
> camelCase variables requires more time for the brain to parse (they're
> easier to write, but take more time to read), please use snake_case like
> the rest of git-svn (and git).
I've switched all variables from camelCase to snake_case. Sorry...old
habit.
> > + # FIXME: We use a combination of git-diff, git-ls-files and git-cat-file
> > + # to divine the state and type of object that was passed in as $path.
> > + # There has to be a better way. Note that only $diffStatus is used
> > + # beyond setting $isDirectory below.
>
> I agree it's pretty ugly. You can probably expand git-runstatus to do
> this. git-commit.sh used to use something like this until git-runstatus
> was added. On the other hand, I'd be content if we dropped support
> for this info entirely since `git-status' is perfectly good.
I thought I saw a patch on the mailing list to remove git-runstatus
after rewriting git-status in C. Because of that, I extracted this
code into a find_file_type_and_diff_status() function in Patch 2/3.
The logic in the function is much easier to follow now, and this also
resulted in cleaning up the cmd_info() function.
> IMNSHO, "URL:" and "Repository Root:" and occasionally "Revision:" (on the
> top-level directory) would be the only useful things this command would
> have to offer.
See above about emulating "svn info" as closely as possible.
> Being able to run something like `git svn info --url <path>'
> to get something like http://svn.foo.org/project/trunk/<path> would be
> nice, too.
See Patch 3/3.
> Please wrap lines at 80 characters. I have a hard time following long
> lines.
Everything has been rewrapped for 80 columns.
> git ls-tree HEAD <filename> will show the mode of a deleted file
Thanks, used this with success in find_file_type_and_diff_status().
> > + rm -rf info gitwc svnwc &&
> All git tests should start you off on a clean trash/ directory...
Removed.
> > + touch -c -r svnwc/symlink-directory gitwc/symlink-directory
> Are -r and -c portable? I remember writing test-chmtime to workaround
> some arguments for touch not being portable.
The touch and cp commands were removed after restructuring the
t/t9117-git-svn-info.sh script to use static expected-* files and
replacing command output using sed.
> Can we expect the output of "svn info" to not change between
> versions? I know "svn status" has changed between versions of
> svn. I'd prefer if we keep the expected.* files hard-coded
> in a test directory and compare those instead. Maybe use sed
> to substitute placeholders for timestamps..
Done.
> Also, git-diff can be used against arbitrary files nowadays, no
> need to rely on a working diff command in the system :)
Done.
> > + cp -p gitwc/added-file svnwc/added-file &&
> I can't remember if cp -p is portable, either...
Fixed (see above).
Dave
next prev parent reply other threads:[~2007-11-21 6:43 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-11-12 16:19 [RFC PATCH] git-svn info: implement info command David D. Kilzer
2007-11-17 22:54 ` Eric Wong
2007-11-21 6:43 ` David D. Kilzer [this message]
2007-11-21 6:43 ` [PATCH 1/3] git-svn: extract reusable code into utility functions David D. Kilzer
2007-11-21 6:43 ` [PATCH 2/3] git-svn info: implement info command David D. Kilzer
2007-11-21 6:43 ` [PATCH 3/3] git-svn: info --url [path] David D. Kilzer
2007-11-21 6:49 ` [PATCH 3/3 v2] " David D. Kilzer
2007-11-21 6:59 ` [PATCH 0/3] Implement git-svn: info David D. Kilzer
2007-11-21 9:18 ` Eric Wong
2007-11-21 9:20 ` Eric Wong
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=1195627399-25209-1-git-send-email-ddkilzer@kilzer.net \
--to=ddkilzer@kilzer.net \
--cc=git@vger.kernel.org \
--cc=normalperson@yhbt.net \
/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;
as well as URLs for NNTP newsgroup(s).