xfs
[Top] [All Lists]

Re: [PATCH 2/2] Add support to RENAME_EXCHANGE flag V3

To: xfs@xxxxxxxxxxx, Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH 2/2] Add support to RENAME_EXCHANGE flag V3
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 13 Nov 2014 09:00:44 +1100
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20141112173245.GA32185@xxxxxxxxxxxxxxxxx>
References: <20141111192922.GC38867@xxxxxxxxxxxxxxx> <20141112173245.GA32185@xxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Wed, Nov 12, 2014 at 03:32:48PM -0200, Carlos Maiolino wrote:
> > > +   if (new_parent) {
> > > +           xfs_trans_ichgtime(tp, target_dp, XFS_ICHGTIME_MOD |
> > > XFS_ICHGTIME_CHG);
> >
> > Long line.
> >
> I think there is no reason to break this line. We should avoid 80+ columns per
> line, but it's not a must, break this line will hurt readability of the code 
> in
> exchange of just a few chars more than 80. I don't think it's worth.

You're not going to win that argument, Carlos: new code wraps at 80
columns. This:

        if (new_parent) {
                xfs_trans_ichgtime(tp, target_dp,
                                   XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
                .....
        }

Is no less readable on a wide terminal than a single line, and it
doesn't end up a mess for people who use 80 column terminals for the
code editing. Please fix it.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>