mlaurent requested changes to this revision. mlaurent added inline comments. This revision now requires changes to proceed.
INLINE COMMENTS > documenttest.cpp:112 > + QCOMPARE( m_document->isDocdataMigrationNeeded(), false ); > + m_document->closeDocument(); > +} closeDocument delete m_document with a deleteLater or we need to delete it before finish method ? > parttest.cpp:785 > + QTemporaryFile nativeFromArchiveFile( QString( "%1/okrXXXXXX.%2" ).arg( > QDir::tempPath() ).arg ( extension ) ); > + QVERIFY( archiveSave.open() ); archiveSave.close(); > + QVERIFY( nativeDirectSave.open() ); nativeDirectSave.close(); new line before archiveSave.close() more readable > document.cpp:4388 > + { > + qWarning() << "Unhandled undo command" << uc; > + } qCWarning(...) > document.cpp:4444 > + > +bool Document::swapBackingFileArchive( const QString &newFileName, const > QUrl & url ) > +{ Remove space before url > document.cpp:4494 > + { > + qWarning() << "Unhandled undo command" << uc; > + } qCWarnign(...) > document.cpp:4729 > + > +Document::OpenResult Document::openDocumentArchive( const QString & docFile, > const QUrl & url, const QString & password ) > +{ coding style : remove space after & > document.h:744 > + * > + * @since 0.20 (KDE 4.14) > + */ 4.14? > document.h:758 > + * > + * @since 0.20 (KDE 4.14) > + */ 4.14? > document.h:771 > + * > + * @since 0.20 (KDE 4.14) > + */ same > document.h:773 > + */ > + bool swapBackingFileArchive( const QString &newFileName, const QUrl > & url ); > + coding style: Remove space before url > document.h:865 > + * > + * @since 0.14 (KDE 4.20) > + */ kde 4.20 ? > documentcommands.cpp:714 > + { > + FormFieldButton *button = dynamic_cast<FormFieldButton > *>(Okular::PagePrivate::findEquivalentForm( newPagesVector[m_pageNumber], > oldFormButton )); > + m_formButtons << button; if button is null do you think that we need to add to QList ? > part.cpp:566 > m_dirtyHandler->setSingleShot( true ); > - connect( m_dirtyHandler, &QTimer::timeout,this, &Part::slotDoFileDirty ); > + connect( m_dirtyHandler, SIGNAL(timeout()),this, > SLOT(slotAttemptReload()) ); > You can still use connect(m_dirtyHandler, &QTimer::timeout, this, [this]() {slotAttemptReload();}) > part.cpp:817 > > - m_saveCopyAs = KStandardAction::saveAs( this, SLOT(slotSaveCopyAs()), ac > ); > - m_saveCopyAs->setText( i18n( "Save &Copy As..." ) ); > - ac->addAction( QStringLiteral("file_save_copy"), m_saveCopyAs ); > - ac->setDefaultShortcuts(m_saveCopyAs, > KStandardShortcut::shortcut(KStandardShortcut::SaveAs)); > - m_saveCopyAs->setEnabled( false ); > + m_save = KStandardAction::save( this, SLOT(saveFile()), ac ); > + m_save->setEnabled( false ); You can use new connect api too > part.cpp:2476 > + QScopedPointer<QTemporaryFile> tempFile; > + KIO::Job *copyJob; // this will be filled with the job that writes to > saveUrl > *copyJob = nullptr; to be sure that it's initialized > annotationmodel.cpp:113 > + if ( !item->annotation ) > + qWarning() << "Lost annotation on document save, something went > wrong"; > + } qCWarning(...) > formwidgets.cpp:89 > + { > + qWarning() << "fwButton is not a QAbstractButton" << fwButton; > + return; qCWarning(...) REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D8642 To: aacid, mlaurent Cc: mlaurent, michaelweghorn, ngraham, #okular, aacid