Details

Reviewers

• ikloecker

Summary

for T7538

requires https://dev.gnupg.org/D617

Test Plan

test with corresponding kleopatra and gpgmepp patches

Diff Detail

Lint

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

@ikloecker would there be any ABI problems with adding the new virtual function here?

• TobiasFella edited the summary of this revision. (Show Details)Thu, Nov 13, 12:53 PM

In D618#6745, @TobiasFella wrote:

@ikloecker would there be any ABI problems with adding the new virtual function here?

Instead of adding new virtuals I have added non-virtual functions that call virtual private functions (NVI pattern). Job has a proper d-pointer since QGpgME 2.0 so this is the way you should go, i.e. add a DeleteJobPrivate. There are plenty of *JobPrivate classes to c&p from.

In D618#6752, @ikloecker wrote:

In D618#6745, @TobiasFella wrote:

@ikloecker would there be any ABI problems with adding the new virtual function here?

Instead of adding new virtuals I have added non-virtual functions that call virtual private functions (NVI pattern). Job has a proper d-pointer since QGpgME 2.0 so this is the way you should go, i.e. add a DeleteJobPrivate. There are plenty of *JobPrivate classes to c&p from.

aah, thanks for the pointer!

I think you are using an outdated working copy of gpgmeqt. I have killed the old jobPrivate stuff with rGPGMEQT056567525fb9: Add d-pointer to Job class. We are using Qt macros now.

src/deletejob.cpp
2	;-)

• TobiasFella updated this revision to Diff 1653.Wed, Nov 19, 4:59 PM

• TobiasFella updated this revision to Diff 1654.

• TobiasFella marked an inline comment as done.

Except for a few minor things this looks good. Thanks!

src/deletejob.cpp
2	Somehow this is still wrong.
src/deletejob.h
40	#include <gpgme++/global.h>
69	This shouldn't be used/needed anymore.
src/deletejob_p.h
44	DeleteJobPrivate is declared as class (as it should be). class DeleteJobPrivate : public JobPrivate
src/multideletejob.h
39	#include <gpgme++/global.h>
58	This should be move into the QGpgME namespace.
src/qgpgmedeletejob.cpp
45–46	This shouldn't be needed.
src/qgpgmedeletejob.h
47	This should be move into the QGpgME namespace.

• TobiasFella updated this revision to Diff 1655.Thu, Nov 20, 12:46 PM

• TobiasFella marked 6 inline comments as done.

• TobiasFella added inline comments.Thu, Nov 20, 12:51 PM

src/deletejob.h
69	are you sure? i can't get it to work without that

• ikloecker added inline comments.Thu, Nov 20, 1:36 PM

src/deletejob.h
69	Yes. It's even wrong because this c'tor would created a DeleteJob without private class which would inevitably crash. The problem is the line make_job_subclass(DeleteJob) in job.cpp. This line needs to be removed and you'll have to define the d'tor yourself in deletejob.cpp (simply as default).

• ikloecker added inline comments.Thu, Nov 20, 1:42 PM

src/deletejob.h
69	And the line #include "moc_deletejob.cpp" needs to be moved to deletejob.cpp.

• TobiasFella updated this revision to Diff 1656.Thu, Nov 20, 1:59 PM

• TobiasFella added inline comments.

src/deletejob.h
69	I have removed the `make_job_subclass(DeleteJob)` line and kept this one. (and added the dtor). Is that what you meant?

• ikloecker added inline comments.Thu, Nov 20, 2:07 PM

src/deletejob.h
69	Removing. Yes. Keeping. No. I did this here and it build without problems with removed `explicit DeleteJob(QObject *parent);`

• TobiasFella added inline comments.Thu, Nov 20, 2:09 PM

src/deletejob.h
69	oh, wait. You're telling me to remove explicit DeleteJob(QObject *parent); not the new ctor, right? now I understand it.

Done now, sorry for the confusion around the contructor

• ikloecker added inline comments.Thu, Nov 20, 2:13 PM

src/deletejob.h
69	Yes. Phabricator's review tool isn't really that great.

You can also delete the `#include "deletejob.h" in job.cpp. Other than that it's ready for merge.

This revision is now accepted and ready to land.Thu, Nov 20, 2:14 PM

• TobiasFella closed this revision.Thu, Nov 20, 3:25 PM

Allow passing flags to DeleteJob
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 1657

src/CMakeLists.txt

src/deletejob.h

src/deletejob.cpp

src/deletejob_p.h

src/job.cpp

src/multideletejob.h

src/multideletejob.cpp

src/qgpgmedeletejob.h

src/qgpgmedeletejob.cpp

Allow passing flags to DeleteJobClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 1657

src/CMakeLists.txt

src/deletejob.h

src/deletejob.cpp

src/deletejob_p.h

src/job.cpp

src/multideletejob.h

src/multideletejob.cpp

src/qgpgmedeletejob.h

src/qgpgmedeletejob.cpp

Allow passing flags to DeleteJob
ClosedPublic
Actions

Revision Contents
Changeset List