Re: [Monotone-devel] [PATCH] Quickie: list duplicates

[Tero Koskinen]

On Thu, 04 Sep 2008 23:27:37 +0200 Thomas Keller wrote:
> 
> Hi Tero!
> 
> > To learn more about Monotone internals, I implemented the "list duplicates"
> > command from the QuickieTasks wiki page.
> 
> Cool, very nice!

Thanks.

> The code looks great overall - just three small things:

I fixed 1) and 2) and probably did same for 3) also, new diff is at
http://iki.fi/tero.koskinen/monotone/monotone_list_duplicates_v2.diff
(and attached in this email).

> 1) Please remove the extra whitespace before opening brackets, f.e.
> "paths.push_back (p);" should just be "paths.push_back(p);"

Oops. I use "f (params);" form in another project and it is hard to
unlearn the habit of placing the extra space after the function name.

> 2) Personally I'd find it easier for the eye to catch if the fileid
> would not be repeated in every line of the output, but would be only
> printed for the very first occurence

Yes, not having fileid repeated looks better. I originally printed the
hash for every file, because it was easier to debug with unix tools
(sort|cut -f1 -d" "|uniq -c|etc...).

> 3) In the manual section I'd add a sentence how this calculation works,
> i.e. what "duplicate files" means to monotone - something like "Two or
> more files are considered duplicates if their contents are equal, i.e.
> have an identical content hash."

I used wording "Two or more files are considered duplicates if the sha1
hashes of their contents are equal." The sentence looks a bit lonely in its
own paragraph, but I couldn't think anything better.

> Other than that its ready to commit. Do you already have commit rights
> on monotone.ca?

I don't have commit rights. To whom should I sent my key?

-- 
Tero Koskinen <address@hidden>

monotone_list_duplicates_v2.diff
Description: Binary data