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.


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