[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>
wrote:
> 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:
