On Thu, Nov 20, 2014 at 10:06:52AM -0600, Endi Sukma Dewata wrote:
On 11/20/2014 12:58 AM, Fraser Tweedale wrote:
>Ping. Who wants to review this :)
Looks short enough, I'll bite. :)
The changes seem to be fine, so it's ACKed.
Thanks Endi; I'll reroll the patch to address those points.
There are some related issues/questions. Feel free to address them as
part
of this ticket, or maybe file a separate ticket.
1. The exception message is not very helpful. It probably should have said
something like:
Max path length cannot be smaller than min path length: <maxLen value>
Yep, I'll do that.
2. According to the ticket the ca-profile-add failed but the profile
actually got added. Suppose there's a real constraint violation I think it
should display the above exception message, and the profile should not be
added.
3. Can we specify any negative value to indicate no min/max, or does it have
to be -1? It should be consistent with the documentation.
Any negative value is considered "no min/max". AFAICT this is
consistent with the documentation. If I'm overlooking something
please point it out.
4. The code seems to assume that the MinPathLen is already added
before
adding MaxPathLen. Suppose the MinPathLen is missing or added later will
this constraint still be validated?
Good catch. I'll perform the validation each time setConfig is
called.
Also, maybe we should keep track the list of unreviewed patches on
the top
of etherpad so we don't miss anything.
+1 ; any objections?
--
Endi S. Dewata