Code review comment for lp:~srazi/qpdfview/some-patches

Revision history for this message
Adam Reichold (adamreichold) wrote :

Hello Razi,

sorry that I do not have to time to review all of this. I did however merge everything that is purely a bug fix into trunk revision 2075. Could you merge trunk back into this branch to make the diff smaller?

From a first look at the code, I noticed that drawAlternativeText should be split into two functions, one with matchedText as they do not share any significant control flow (and common setup could be factored out into a helper function).

Also signal names are usually formulated in the passive voice, i.e. requestAppendToBookmarkComment should rather be appendToBookmarkRequested.

Best regards,
Adam

« Back to merge proposal