Code review comment for lp:~komar007/qpdfview/unique_instances

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

Hello Michał,

Tanks for your contribution! It seems a useful feature and I am very much for merging the branch. I hope I'll have some time tonight for a thorough review. Just some first remarks:

* Could you leave out the translation updates? I'll run "lupdate" when this is inside trunk. It just makes the diff larger.

* Your interpretation of the database table name is correct, at least what you described was my plan. :-) But since there has been no release using the "v1" table as of now, you can leave out the reimport of the "v1" table. This is all happing in trunk and so we do not have to give guarantees to the outside world yet. (But the new name is still correct, so we do not try to use the old table.)

* If you don't mind, I would add a copyright notice for you to the modified source files after merge and maybe make some smaller style changes. (Or you add the copyright notice yourself using your preferred mail address.)

Best regards, Adam.

« Back to merge proposal