[haiku-development] Re: Final Set*UIColor Patch, Version 3e

  • From: Adrien Destugues <pulkomandy@xxxxxxxxx>
  • To: haiku-development@xxxxxxxxxxxxx
  • Date: Fri, 4 Dec 2015 22:17:33 +0100

On Fri, Dec 04, 2015 at 08:47:11PM +0000, Fredrik Holmqvist wrote:

Why still keep this out of tree?
Isn't it easier to fix the remaining issues in tree?
We have people who can help improve it there. Is our git a development tree
or a golden master where only perfect code enter?

I prefer a development tree where we can collaborate.

/Fredrik Holmqvist, TQH

You can take the responsability of reviewing and merging, then?

--
Adrien.


fre 4 dec. 2015 18:26 looncraz <looncraz@xxxxxxxxxxx> skrev:

I have an available time window of three days next week, starting
Tuesday. I would like to use a couple of these days to address any
remaining blocking issues for final approval and committing of my patch.

For this to work, I will require answers for the questions I have
previously asked, as well as feedback regarding my explanations in
regards to various implementation details (such as the value of
DelayedMessage and keeping BColorMap (as well as keeping it as public)).

The current version of the patch, version 3g, has the following changes
(from 3e/f):

* BColorMap stores rgb_color as a B_RGB_32_BIT_TYPE rather than as int32
- This changes the format of the B_COLORS_UPDATED message as well
* BColorMap::SetUIColors() calls to the Desktop thread directly.
- This ends the practice of spamming set_ui_color, though that remains
safe.
* B_FOLLOW_DEFAULT has been renamed B_FOLLOW_LEFT_TOP
* A few minor issues (dead code, encapsulation, etc.) resolved

Questions that need to be addressed:

1. Is DelayedMessage acceptable given my previous explanations?
2. If DelayedMessage is not acceptable, what is the alternative offered?
3. Should BColorMap remain?
4. If not, what is the alternative? I see few worthwhile options (see $6)
5. If BColorMap should remain, should it remain a part of the public API?
6. Should BMessage get Add/SetColor(color_which, rgb_color) methods?
7. Is it possible to find a View by token in the app_server? If so, how?
8. If I were to implement client-side delayed messaging, what form would
be best?
- BMessenger::SendAtTime(), seems reasonable, but what about merging
messages?

These are unaddressed questions just from my previous response to Axel's
review. This is the fourth time I've received no response to my
questions or comments. There will not be a fifth.

--The loon



Other related posts: