On Mon, Jan 28, 2013 at 5:59 AM, C. Michael Pilato <[email protected]> wrote:
> My only remaining minor nit is the variable name itself.  First, other
> variable names in this script use underscores to separate words:  for_repos,
> label_from, etc.  Secondly, it's really only the basename of the directory
> that's being substituted in, and I think the variable name should carry that
> information.  (I had to read your docs to make sure I wouldn't wind up with
> "[svn-/path/to/my/repository commit]".)  So, I'd suggest using
> "repos_basename" rather than "repodir".  Again, it's a minor thing -- and
> one that the patch applier can probably make on your behalf -- but if the
> name caused me some initial confusion, it'll probably do the same to someone
> else, too.

I almost said something about the name too when I reviewed it.  I let
it go since you could easily say repodir means the name of the
directory that the repo is in.  I'd have gone with repos_name
personally.

But at this point I feel like we're painting the bikeshed.

Reply via email to