D8642: Rework saving of annotations and form data

Albert Astals Cid noreply at phabricator.kde.org
Wed Nov 15 10:20:22 UTC 2017


aacid marked 7 inline comments as done.
aacid added inline comments.

INLINE COMMENTS

> mwolff wrote in documenttest.cpp:105
> can we test the actual migration, too? i.e. instead of pretending, actually do it?

Should be doable, i'll have a look.

> mwolff wrote in documenttest.cpp:112
> shouldn't this have an `QEXECTED_FAIL`, considering that in principle the annotations should have been migrated?

i don't understand why we would use  QEXECTED_FAIL here, yes the annotations have been migrated so we compare docdata migration to false, at most we could use a qverify since one could argue qcompare with bools is weird, but why qexpected_fail?

> mwolff wrote in parttest.cpp:827
> this overrides the helper from above, is this desired? do you need a stack of helpers? or do you maybe miss a wait step before (signal spy?) to ensure the previous dialog has been shown and closed?

Yes it overrides the helper from above.

Yes it is desired since the helper from above was already used in saveAs

No i don't need a stack of helpers

No i don't need a wait step since the helper destructor already ensures the previous dialog was shown and closed

> mwolff wrote in parttest.cpp:854
> dito, this again potentially overrides the close helper, no?

Same answer as above, this does exactly what it has to do.

> mwolff wrote in generator.h:296
> whitespace issue after &

You don't really want to look at all the whitespace issues the okular code has.

> mwolff wrote in page.cpp:502
> ws issues like above

No, this is actually the okular style (most of the time)

> mwolff wrote in page.h:256
> whitespace issue after &

don't complain about whitespace issue, there's too many styles to "fix" it.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D8642

To: aacid, mlaurent, rkflx
Cc: mwolff, rkflx, lueck, mlaurent, michaelweghorn, ngraham, #okular, aacid
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20171115/4b4248cc/attachment-0001.html>


More information about the Okular-devel mailing list