On 2016/09/19 2:27 PM, Jan-Marek Glogowski wrote:

Some random low priority comments:

In "Just walk the task list once per timeout"
The evaluate_entry label doesn't make the code any easier to read.

So you're suggestion is? Introduce an other level of indention for the
if? The while loop is short enough and the goto labels are just used in
there. If you think it improves readability, I can change it.

No, just inline that single line of code at the evaluate_entry label and then 
jump to the exit label.
But that's just my taste, if you disagree, feel free to ignore.

In "Reorganize Scheduler priority classes"
 (1) you haven't used a consistent mapping from old to new - sometimes
LOW maps to HIGH_IDLE, sometimes HIGHEST maps to HIGH_IDLE. Perhaps
there is a reason for this?

From naming and code reading I tried to guess, if the idle should be
processed before drawing. As I wrote I would like to get a comment or
annotation for all non-default priorities, i.e. all calls to "SetPriority".

I have no idea, if my guesses are correct, just that mail merge is much
faster now and LO is still usable while the mail merge runs. Actually
you can see the slowdown of mail merge when doing stuff in LO :-)

OK, just checking, because from the commit message it looks like a mechanical change, but then the mapping was not consistent.

(2) I would call HIGH_IDLE, either PRE_RESIZE or BEFORE_RESIZE, because
it's not any kind of IDLE anymore. I would call DEFAULT_IDLE just IDLE.

I just took the priority names from glib. I would keep the DEFAULT_IDLE,
or rename DEFAULT to TIMER? OTOH the Scheduler class sets the default

Ah, then probably leave them alone, if they map to something we already use.

LibreOffice mailing list

Reply via email to