qtubuntu-print

Merge lp://staging/~ahayzen/qtubuntu-print/empty-branch-for-bileto into lp://staging/qtubuntu-print

  1. empty-branch-for-bileto
  2. Merge into trunk
Proposed by Andrew Hayzen
Status: Merged
Approved by: Michael Sheldon
Approved revision: 26
Merged at revision: 2
Proposed branch: lp://staging/~ahayzen/qtubuntu-print/empty-branch-for-bileto
Merge into: lp://staging/qtubuntu-print
Diff against target: 1359 lines (+732/-306)
23 files modified
.bzr-builddeb/default.conf (+2/-0)
CMakeLists.txt (+90/-0)
COPYING (+165/-0)
constants.h (+34/-0)
debian/changelog (+2/-2)
debian/control (+19/-10)
debian/copyright (+14/-24)
debian/qtubuntu-print.install (+1/-0)
debian/qtubuntu-print.sh (+7/-0)
debian/rules (+7/-20)
i18n.cpp (+40/-0)
i18n.h (+35/-0)
main.cpp (+22/-19)
pdf-plugin.pro (+0/-28)
po/CMakeLists.txt (+30/-0)
po/qtubuntu-print.pot (+36/-0)
qtubuntu-print.json (+1/-1)
qubuntuprintdevice.cpp (+28/-18)
qubuntuprintdevice_p.h (+22/-19)
qubuntuprintengine.cpp (+85/-81)
qubuntuprintengine_p.h (+24/-22)
qubuntuprintsupport.cpp (+49/-45)
qubuntuprintsupport_p.h (+19/-17)
To merge this branch: bzr merge lp://staging/~ahayzen/qtubuntu-print/empty-branch-for-bileto
Reviewer Review Type Date Requested Status
Michael Sheldon (community) Approve
Ken VanDine packaging Approve
Review via email: mp+312858@code.staging.launchpad.net

Commit message

* Initial commit

Description of the change

* Empty commit

To post a comment you must log in.
lp://staging/~ahayzen/qtubuntu-print/empty-branch-for-bileto updated
3. By Andrew Hayzen

* Set the section to libs

4. By Andrew Hayzen

* Fix build depends, qmake etc for building in clean env

5. By Andrew Hayzen

* Change peer's ID to ubuntu-printing-app

6. By Andrew Hayzen

* Rename everything from pdfsupport/pdfplugin to qtubuntu-print

7. By Andrew Hayzen

* Tweaks to content-hub exporting

8. By Andrew Hayzen

* Define printing app id

Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

The user visible strings in: http://bazaar.launchpad.net/~ahayzen/qtubuntu-print/empty-branch-for-bileto/view/head:/qubuntuprintdevice.cpp need to be translatable

I think you should probably also add a dependency on ubuntu-printing-app since the plugin isn't usable without it.

It might be better to use your Canonical email address for the Maintainer and changelog entries?

review: Needs Fixing
Revision history for this message
Andrew Hayzen (ahayzen) wrote :

I think the fileName that is generated should have a random hash in it, to prevent two identical filenames if two print requests were done rapidly.

In closePrintDevice, the file should probably only be exported when the output filename has been set by us.

I have offline changes that attempt to disable options like copies, collate when using the printing service (as they don't make sense). Will check if they work and if they do include them.

lp://staging/~ahayzen/qtubuntu-print/empty-branch-for-bileto updated
9. By Andrew Hayzen

* Change use of file to use QTemporaryFile
* Only export to content-hub if the outputFilename was set by us
* Update name in copyright to qtubuntu-print
* Update changelog and control to use canonical email address

Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

The #ifdefs for the headers are still named based on the PDF plugin (QPDFSUPPORT_H, QPDFENGINE_H, QPDFDEVICE_H), they should be updated to reflect the new filenames

review: Needs Fixing
lp://staging/~ahayzen/qtubuntu-print/empty-branch-for-bileto updated
10. By Andrew Hayzen

* Fix ifdef names missed in migration to qtubuntu-print

11. By Andrew Hayzen

* Migrate to CMake
* Get translations working
* Ensure use of fully qualified import names

12. By Andrew Hayzen

* Add dh-translations and pkg-config as depends
* FIx debian build not being able to find main.moc

Revision history for this message
Andrew Hayzen (ahayzen) wrote :

I believe I have fixed all the issues, please rereview :-)

Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

All looks good to me now :)

review: Approve
lp://staging/~ahayzen/qtubuntu-print/empty-branch-for-bileto updated
13. By Andrew Hayzen

* Change ubuntu-printing-app to be a recommends of qtubuntu-print

14. By Andrew Hayzen

* Show warning widget when transfer could not be created due to missing ubuntu-printing-app

15. By Andrew Hayzen

* Change dialog title to "Printing Failed"

16. By Andrew Hayzen

* If the transfer state is aborted then it has failed and don't try to charge

Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

Changes work nicely :)

review: Approve
Revision history for this message
Ken VanDine (ken-vandine) wrote :

Packaging review:

debian/copyright
  * GPL-3.0+ should really be GPL-3.0. However this is a library so I think LGPL is probably more suitable than GPL, but I'm not a lawyer.
  * If the license and the copyright is the same, you don't need the Files: debian/* section at all.
  * Remove the comments at the bottom, that was meant to be template documentation
  * You need to include a copy of the license with the source, copy the correct license file to COPYING in the root of the project

debian/rules:
  * Please remove any unused code in the file

You need to use split mode to get the sources properly, please add the following file to the project. Look at content-hub for an example:
cat .bzr-builddeb/default.conf
[BUILDDEB]
split = True

review: Needs Fixing (packaging)
lp://staging/~ahayzen/qtubuntu-print/empty-branch-for-bileto updated
17. By Andrew Hayzen

* Add bzr-builddeb config
* Add COPYING to the root of the project
* Remove comments from copyright and rules that aren't needed
* Remove the debian/* section in the copyright it isn't needed
* Change debian/copyright from GPL-3.0+ to GPL-3.0

Revision history for this message
Andrew Hayzen (ahayzen) wrote :

Fixed the issues you have found, I'm going to check with Bill which license he would prefer.

Please rereview :-)

One other thing I wondered is now that we are able to detect when a peer doesn't exist this could first try to export to the snap peer snap.ubuntu-printing-app.ubuntu-printing-app and then to the debian peer ubuntu-printing-app. If both fail then it could show the dialog, what do you think? Also I'm going to check with Bill to see what his plan was regarding how ubuntu-printing-app should be distributed (eg as it's own snap or within the unity8 one).

lp://staging/~ahayzen/qtubuntu-print/empty-branch-for-bileto updated
18. By Andrew Hayzen

* Add script which is installed under /etc/profile.d/ to set QT_PRINTER_MODULE when running under unity8 deb edition

19. By Andrew Hayzen

* Change to using LGPL instead of GPL

20. By Andrew Hayzen

* Change the name to "Ubuntu Printing Service"
* Create a constants.h file which holds consts used across the library
* Regenerate the pot

21. By Andrew Hayzen

* Pass APP_ID as argument so it is not translated

22. By Andrew Hayzen

* Add constants.h file to bzr

Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

Changes look good to me :)

review: Approve
lp://staging/~ahayzen/qtubuntu-print/empty-branch-for-bileto updated
23. By Ken VanDine

* Packaging cleanup, mostly just quieting down lintian. I also ran wrap-and-sort to format debian/control to match our other packages

24. By Andrew Hayzen

* Add missing define to prevent loading constants.h twice

25. By Andrew Hayzen

* Re-add qt5-default otherwise qmake command embedded in cmake fails with "qmake: could not find a Qt installation of ''"

26. By Andrew Hayzen

* Remove qt5-default and pass -qt=qt5 to qmake

Revision history for this message
Ken VanDine (ken-vandine) wrote :

Packaging changes look good now

review: Approve (packaging)
Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

Changes look good

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: