xfs
[Top] [All Lists]

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

To: xfs@xxxxxxxxxxx
Subject: Re: [PATCH 2/2] Add support to RENAME_EXCHANGE flag V3
From: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
Date: Thu, 13 Nov 2014 11:04:18 -0200
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20141112220044.GB23575@dastard>
Mail-followup-to: xfs@xxxxxxxxxxx
References: <20141111192922.GC38867@xxxxxxxxxxxxxxx> <20141112173245.GA32185@xxxxxxxxxxxxxxxxx> <20141112220044.GB23575@dastard>
User-agent: Mutt/1.5.23 (2014-03-12)
On Thu, Nov 13, 2014 at 09:00:44AM +1100, Dave Chinner wrote:
> 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.

Ok, I don't mind, just gave my point of view :) I'm going to change this in the
next version.

-- 
Carlos

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