Re: [Monotone-devel] Re: new restrictions branch for including required parents

[Thomas Keller]

Am 18.05.10 09:26, schrieb Thomas Keller:
> Am 18.05.2010 04:56, schrieb Derek Scherger:
>> On Mon, May 17, 2010 at 2:26 AM, Thomas Keller <address@hidden> wrote:
>>
>>> Ok, as long as you have documented that for the revert command,
>>> everything should be fine. A short note in NEWS shouldn't hurt as well.
>>>
>>
>> Done and ready to merge. Any objections?
> 
> Sorry, I forgot to review it last night - give me one more day :)

Ok here we go - code looks very good overall, some minor questions:

1)

+            case restricted_path::required:
+            case restricted_path::excluded_required:
+              if (path_depth == 0)
+                {
+                  L(FL("implicit include of nid %d path '%s'") %
current % fp);
+                  return true;
+                }
+              else
+                {
+                  return false;
+                }
+              return true;

If I followed the code correctly this logic is basically there to avoid
the exclusion of the root directory '', right? Would it make sense to
give this special node the correct node / path status right from the
start and therefor "move" this logic upwards? Also, the last return true
can never be reached, it should probably be

+            case restricted_path::required:
+            case restricted_path::excluded_required:
+              if (path_depth == 0)
+                {
+                  L(FL("implicit include of nid %d path '%s'") %
current % fp);
+                  return true;
+                }
+              return false;

2) The test case restricted_diff_bug should get a better name, its no
longer a bug :)

Other than that I think its ready to get merged - I suspect Stephe has
to make his undrop command equally use explicit_includes, just like
revert, right?

Thomas.

-- 
GPG-Key 0x160D1092 | address@hidden | http://thomaskeller.biz
Please note that according to the EU law on data retention, information
on every electronic information exchange might be retained for a period
of six months or longer: http://www.vorratsdatenspeicherung.de/?lang=en

signature.asc
Description: OpenPGP digital signature