[haiku-bugs] Re: [Haiku] #7445: app_server DecorManager + DecorInfo patch
- From: "stippi" <trac@xxxxxxxxxxxx>
- Date: Fri, 15 Apr 2011 08:03:26 -0000
#7445: app_server DecorManager + DecorInfo patch
----------------------------------+-------------------------------------
Reporter: looncraz | Owner: axeld
Type: enhancement | Status: new
Priority: normal | Milestone: R1
Component: Servers/app_server | Version: R1/Development
Resolution: | Keywords: DecorManager, decorator
Blocked By: | Blocking:
Has a Patch: 1 | Platform: All
----------------------------------+-------------------------------------
Comment (by stippi):
Thanks for the patch and also for using the proper channels! :-) The
changes are definitely fine and welcome. If you feel motivated enough, you
could do another iteration of coding style cleanup, otherwise I would do
it before applying the patch. The biggest inconsistencies are with
parenthesis indentation in the files you wrote from scratch. Some more
specific remarks which stuck from reading the patch:
* Please keep the asterisk style consistent (DecorManager.h and others).
We prefer "Type* name". In a header, the asterisk would go by the type as
well, with tabbing until the name.
* Please keep opening parenthesis indentation consistent. The opening one
goes on the same line, except for funtions and blocks. In the files
written from scratch, you have both inconsistent indentation and put the
opening one on a new line...
* Please don't change the copyright style. So instead of doing this:
{{{
/*
- * Copyright 2007, François Revol, revol@xxxxxxxx
* Distributed under the terms of the MIT license.
*
+ * Authors:
+ * François Revol <revol@xxxxxxx>
+ * Joseph "looncraz" Groover <looncraz@xxxxxxxxxxx>
+ */
}}}
(which BTW removes the actual Copyright line), you should do this:
{{{
/*
+ * Copyright 2011, Joseph "looncraz" Groover <looncraz@xxxxxxxxxxx>
* Copyright 2007, François Revol, revol@xxxxxxxx
* Distributed under the terms of the MIT license.
*/
}}}
* Other than the coding style violations, the functionality seems fine to
me, but I have only skimmed over a lot of chunks of the patch. Your
solution with the preview window is certainly "creative" :-). I would
personally have liked it better, if a decorator could render into a bitmap
and that would be displayed in Appearance. Each decorator side by side,
like in the GNOME appearance panel. So the user just clicks on the one he
thinks looks good.
--
Ticket URL: <http://dev.haiku-os.org/ticket/7445#comment:2>
Haiku <http://dev.haiku-os.org>
Haiku - the operating system.
Other related posts:
