FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Cluster Development

 
 
LinkBack Thread Tools
 
Old 02-01-2008, 04:24 PM
"J. Bruce Fields"
 
Default NLM add resume procfs file

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.

--b.
 

Thread Tools




All times are GMT. The time now is 05:08 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org