Re: [PATCH 03/19] mkfs: Sanitise the superblock feature macros

Jan Tulak
2016-04-07T11:54:02Z

[-- Attachment #1.1: Type: text/plain, Size: 1699 bytes --]

On Wed, Apr 6, 2016 at 11:01 PM, Dave Chinner <david@fromorbit.com> wrote:

> On Wed, Apr 06, 2016 at 11:12:21AM +0200, Jan Tulak wrote:
> > On Fri, Apr 1, 2016 at 4:05 AM, Eric Sandeen <sandeen@sandeen.net>
> wrote:
> > > On 3/24/16 6:15 AM, jtulak@redhat.com wrote:
> > > if (sb_feat.finobt && sb_feat.finobtflag) {
> > > fprintf(stderr,
> > > _("warning: finobt not supported without CRC support, disabled.\n"));
> > > }
> > > sb_feat.finobt = 0;
> > >
> > > would suffice as before, no? Meh. Not a big deal I guess....
> > >
> >
> > ​Changed. Honestly, I don't like the strings starting at the beginning of
> > the line, because it breaks the indentation flow, but the rest of the
> code
> > uses this style, so I should stick to it.​
>
> There's good reason for doing this - it makes it easy to grep the
> source code for a specific error that has been emitted. Indentation
> is useful for demonstrating logic flow, but it's harmful when it
> results in strings you might want to find being split up over
> multiple lines.
>

​Mmm, yeah, a valid point. It is caused by the 80 chars limit, but there
are reasons for that too​...

> > Thank you for the review. I will wait a little longer if someone spots
> > something more, before sending an updated patchset.​ :-)
>
> Just send it - I almost got to pulling in this version and
> fixing the various comments directly myself yesterday....
>

​OK. I will fix what Eric submitted in the mean time, test it to be sure I
didn't broke anything and send.

Cheers,
Jan

--
Jan Tulak
jtulak@redhat.com / jan@tulak.me

[-- Attachment #1.2: Type: text/html, Size: 3445 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]