[haiku-development] Re: [patch] Show partition types in DriveSetup

  • From: "Ingo Weinhold" <ingo_weinhold@xxxxxx>
  • To: haiku-development@xxxxxxxxxxxxx
  • Date: Mon, 16 Apr 2012 20:59:23 +0200

John Scipione wrote:
> On Mon, Apr 16, 2012 at 1:33 PM, José Manuel <jmfo1982@xxxxxxxxx> wrote:
> > This is my first patch for Haiku, it modifies DriveSetup application
> > so it shows (again) the partition type, for partitions that had no
> > recognized file system (such as Linux swap, or a newly created
> > partition for BeFS).
> >
> > It will show the partition type in the Filesystem column, but this
> > time enclosed in parentheses as per the suggestion in:
> > http://dev.haiku-os.org/ticket/4235#comment:5
> >
> > This way, in cases where the partition type name matches a Filesystem
> > name (like "Be File System") it will be obvious whether the partition
> > is initialized or not.
> 
> First off, as a general rule, please don't send attachments on emails to
> this list. Although in this case the file is quite small so it is not much
> of a problem. You can attach the patch to the ticket and link that in the
> email.

Sending a patch to the development mailing list is fine, particularly when you 
want to discuss it. AFAIK there's never been a rule against that. However, as 
Adrien replied, attaching it to a ticket is preferred, if you want it to be 
applied eventually.

> Commenting on the ticket is better then sending an email to this list as
> well since a lot of people are subscribed to this list and many of them
> don't care about this or that particular bug. When you comment on a bug
> report it will automatically email everyone following that bug.

I suspect for a lot of topics raised on this mailing list, you find a lot of 
people who are not interested in it. :-)

> That being said I find this line to be particularly... interesting.
> 
>  BString partitionType = (BString) "(" << partition->Type() << ")";
> 
> I wouldn't think that casting to a BString like that would work, perhaps
> I'm wrong and that is okay but I'd prefer to see something like this:

BString has a cast constructor for "const char*", so this is perfectly valid 
and equivalent to calling the constructor (BString("(")). The latter is 
preferrable, though. According to our coding style there should be no space 
after the cast operator, BTW.

>  BString partitionType = BString("(");
>  partitionType << partition->Type() << ")";

I like the single line version better.

> or better yet:
> 
>  BString partitionType = BString(partition->Type());
>  partitionType.Prepend("(");
>  partitionType.Append(")");

That reduces the readability IMO. Using SetToFormat() would also be an 
alternative with good readability.

CU, Ingo

Other related posts: