Re: [PATCH 0/2] xfsdump whitespace changes

[Jan Tulak <jtulak@redhat.com>]

On Fri, Nov 2, 2018 at 11:57 PM Dave Chinner <david@fromorbit.com> wrote:
>
> On Fri, Nov 02, 2018 at 05:54:59PM +0100, Jan Tulak wrote:
> >
> > Update: I found a reasonably working code prettifier with kernel-style
> > config. The disadvantage is that I'm not really able to split the
> > patches by type of change (indentation, spaces inside of parentheses,
> > vertical align...) and it did it all at once, but hopefully that is
> > not a big issue. You can look at it here [1] - it is 2,4MB in size of
> > the patch files (only the formatting changes) and I want to check it a
> > bit better before sending it here as emails, whether there is no new
> > warning in gcc, etc. But it compiles and passes xfstests, so feel free
> > to peek at it - I will be back online in the Monday.
> >
> > [1] https://github.com/jtulak/xfsdump/tree/for-review  (git clone
> > https://github.com/jtulak/xfsdump.git)
>
> IMO, it's largely unreviewable because not only does whitespace
> change, so does the way the code is laid out.
>
> e.g it removes {} around single line if/for/while, so that could
> introduce bugs if there are multi-expression macros that aren't
> correctly encapsulated (and there are!).

I didn't realize that possibility. Fixing...

>
> And that's really hard to see in amongst all the indenting changes,
> the whitespace removal, etc.  I'd much prefer "one type of
> change at a time" patches because they are much easier to review.

Yeah, I understand. Let's see what I can do... Anyway, thanks for the feedback.

Cheers,
Jan



-- 
Jan Tulak