Code review comment for lp:~cimi/overlay-scrollbar/new-thumb-functionalities

Revision history for this message
Ted Gould (ted) wrote :

In general, I'm a little uncomfortable with all the bit twittling for the state variable. I think that you should put that into its own function. Two reasons, it's easy to mess up. And it seems to be likely someplace you'll want to put a g_debug() statement when things are going wrong.

Also, the diffs would be a lot smaller if the name changes weren't included with the logic changes. I think next time you should split those out into two different merge requests.

Other than that, it was a big diff! :-) I didn't see anything on a general read through.

review: Approve

« Back to merge proposal