[haiku-development] Re: app_server patch

  • From: Ingo Weinhold <ingo_weinhold@xxxxxx>
  • To: haiku-development@xxxxxxxxxxxxx
  • Date: Wed, 13 Apr 2011 16:38:14 +0200

On 2011-04-13 at 09:47:35 [+0200], Joseph Groover <looncraz@xxxxxxxxxxx> 
> I have completed a (hopefully) fully working patch along the lines
> mentioned before.
> You can find it at the following URL along with some more detailed
> information for those interested.
> http://looncraz.tripod.com/haiku/
> At this point I haven't created a ticket, but will do so when I have time.
> This patch is not yet finalized and is based on changes I made to
> r41206.  The patch file is ~500kb.

Mentioning such a figure doesn't help to motivate anyone to look at it. You 
should at least also mention that most of that weight stems from an added 
bitmap resource and that the code part of the patch is only around 2k 
lines. I haven't looked at why you added the bitmap. Your patch summary 
doesn't say so.

> ALL opinions wanted!

First of all, most people prefer not to have to download and unzip a patch 
before being able to look at it. Also, Trac's patch viewer is rather nice 
IMO, so I'd recommend using it for larger patches (anything longer than a 
few hundred lines).

I'm not an app server guy, so I'll only comment on the build system part: 
The Decorator rule you introduced is mostly a copy of the Addon rule. 
There's no need for the code duplication; the Addon rule could just be 
invoked. Other than that there's nothing that makes the Decorator rule 
decorator specific. I believe the Addon rule is lacking a parameter for 
resources only because there have been only few add-on's that need them (if 
there are any at all) and calling the AddResources rule manually isn't much 
trouble either. I don't see any reason not to add such a parameter, though.

CU, Ingo

Other related posts: