On Wed, Jan 30, 2008 at 11:43:50AM -0500, Wendy Cheng wrote:
> J. Bruce Fields wrote:
>>> + int rc;
>>> + rc = failover_parse_ip(file, buf, size, &server_ip);
>>> + if (rc < 0)
>>> + return rc;
>>> return nlmsvc_failover_ip(server_ip);
>> Looks great, but it would fit more logically with the previous patch.
>> (If you know you're going to end up using this code in two places, may
>> as well write it that way from the start.)
> The original unlock patch did have a shared routine for this purpose.
> After review, its code structure got changed a little bit. Since the
> revised version has non-trivial amount of testing efforts behind it, I
> think it is better to do the change here, instead of the well-tested
> unlock patch.
> On the other hand, I cut the resume patch into three pieces mostly for
> review purpose. Do you think it would be easier (for your git tree
> works) that I combine these three small patches into a big resume patch
> after all the review comments are incorporated into the code ?
As long as they each compile and run without introducing any new bugs
(even temporarily) along the way, then I'll almost always prefer more
smaller patches to fewer larger ones.