On 17/06/12 19:28, Carlos R. Mafra wrote:

> On Fri, 15 Jun 2012 at 23:06:18 +0200, Rodolfo García Peñas wrote:
> 
>> Subject: [PATCH] Icon painting moved to makeAppIconFor()
>>
>> The icon painting is moved to the function makeAppIconFor()
>> including the check for no_appicon.
>>
>> wAppIconCreate is now static because is only used in makeAppIconFor()
>> The function is moved above to avoid the prototype creation.
> 
> No, moving the position of a function inside the same file to avoid
> the prototype is not a good reason. It makes the patch artificially 
> bigger and does not set a good precedent. 

Yes, but I thougth that the final code is better without the prototype.

> Could you please respin the patch without the moving?

Yes, of course. I just sent it.

> Furthermore, does any of your later patches depend on your
> "experimental" ones?

No. The experimental patch was only a try. I am happy with the comments
about it, and for this reason I made this new patches. This patches
includes the removal of the extractIcon function and the movement of the
code of icon creation in application.c to appicon.c (this patch, with
creation and painting moved to makeAppIconFor()).

We don't need (at least now) split the creation and paint, because I did
it in the makeAppIconFor using the check of the no_appicon, as you recomend.

I think that these two patches do the same work that the experimental
patches, but with better code.

I will think a little more about the icon creation and painting, I think
the work is not finished yet, but is better now.

Again, thanks for your comments and your patch review.

Best Regards,
kix.

> 
>> ---
>>  src/appicon.c     |   93 
>> ++++++++++++++++++++++++++---------------------------
>>  src/appicon.h     |    1 -
>>  src/application.c |    4 ---
>>  3 files changed, 46 insertions(+), 52 deletions(-)
> 
> 



--
To unsubscribe, send mail to [email protected].

Reply via email to