Ben Reser wrote on Mon, Jan 28, 2013 at 09:25:30 -0800: > 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.
I'll commit with cmpilato's tweaks (indentation and varname) as I agree with them both.
