← Back to team overview

apport-hackers team mailing list archive

Re: [Merge] lp:~kubuntu-packagers/apport/qt5 into lp:apport

 

Review: Needs Fixing



Diff comments:

> === modified file 'kde/apport-kde'
> --- kde/apport-kde	2015-02-06 07:42:03 +0000
> +++ kde/apport-kde	2015-03-25 13:19:48 +0000
> @@ -1,7 +1,8 @@
>  #!/usr/bin/python
>  
> -'''KDE 4 Apport User Interface'''
> +'''Qt 5 Apport User Interface'''
>  
> +# Copyright (C) 2015 Harald Sitter <sitter@xxxxxxx>
>  # Copyright (C) 2007 - 2009 Canonical Ltd.
>  # Author: Richard A. Johnson <nixternal@xxxxxxxxxx>
>  #
> @@ -17,16 +18,10 @@
>  
>  try:
>      import apport
> -    from PyQt4.QtCore import *
> -    from PyQt4.QtGui import (QDialog, QLabel, QCheckBox, QRadioButton,
> -                             QTreeWidget, QTreeWidgetItem, QFileDialog,
> -                             QDialogButtonBox, QProgressBar, QLineEdit,
> -                             QPushButton, QIcon, QPainter, QMovie)
> -    from PyQt4 import uic
> -    from PyKDE4.kdecore import (ki18n, KAboutData, KCmdLineArgs,
> -                                KLocalizedString)
> -    from PyKDE4.kdeui import (KApplication, KMessageBox, KIcon,
> -                              KStandardGuiItem)
> +    from PyQt5.QtCore import *

FYI: upstream PyQt discourages use of such imports as they spam the global namespace, and also make it impossible to use tools like PyFlakes which can detect undefined variables (in one of my apps, I found some real bugs with pyflakes after I removed such import).

Explicitly specifying names to import is better.

> +    from PyQt5.QtGui import *
> +    from PyQt5.QtWidgets import *
> +    from PyQt5 import uic
>      import apport.ui
>      from apport import unicode_gettext as _
>      import sip
> @@ -396,13 +391,13 @@
>              self.progress.set()
>              self.progress.show()
>  
> -        KApplication.processEvents()
> +        QApplication.processEvents()
>  
>      def ui_pulse_info_collection_progress(self):
>          if self.progress:
>              self.progress.set()
>          # for a spinner we just need to handle events
> -        KApplication.processEvents()
> +        QApplication.processEvents()
>  
>      def ui_stop_info_collection_progress(self):
>          if self.progress:
> @@ -412,7 +407,7 @@
>              self.dialog.movie.stop()
>              self.dialog.spinner.hide()
>  
> -        KApplication.processEvents()
> +        QApplication.processEvents()
>  
>      def ui_start_upload_progress(self):
>          self.progress = ProgressDialog(
> @@ -427,7 +422,7 @@
>              self.progress.set(progress)
>          else:
>              self.progress.set()
> -        KApplication.processEvents()
> +        QApplication.processEvents()
>  
>      def ui_stop_upload_progress(self):
>          self.progress.hide()
> @@ -504,27 +499,10 @@
>      if not os.environ.get('DISPLAY'):
>          apport.fatal('This program needs a running X session. Please see "man apport-cli" for a command line version of Apport.')
>  
> -    appName = 'apport-kde'
> -    catalog = 'apport'
> -    programName = ki18n(b'Apport KDE')
> -    version = '1.0'
> -    description = ki18n(b'KDE 4 frontend for the apport crash report system')
> -    license = KAboutData.License_GPL
> -    copyright = ki18n(b'2009 Canonical Ltd.')
> -    text = KLocalizedString()
> -    homePage = 'https://wiki.ubuntu.com/Apport'
> -    bugEmail = 'kubuntu-devel@xxxxxxxxxxxxxxxx'
> -
> -    aboutData = KAboutData(appName, catalog, programName, version, description,
> -                           license, copyright, text, homePage, bugEmail)
> -
> -    aboutData.addAuthor(ki18n(b'Richard A. Johnson'), ki18n(b'Author'))
> -    aboutData.addAuthor(ki18n(b'Michael Hofmann'), ki18n(b'Original Qt4 Author'))
> -
> -    KCmdLineArgs.init([''], aboutData)
> -
> -    app = KApplication()
> -    app.setWindowIcon(KIcon('apport'))
> +    app = QApplication(sys.argv)
> +    app.applicationName = 'apport-kde'

Here, you overwrite a method, which makes no sense:

>>> app = QApplication([])
>>> app.applicationName
<built-in method applicationName of QApplication object at 0xb71f9194>

Probably you wanted to use app.setApplicationName('apport-kde') instead.
The same in next two lines.

> +    app.applicationDisplayName = _('Apport')
> +    app.windowIcon = QIcon.fromTheme('apport')
>  
>      UserInterface = MainUserInterface()
>      sys.exit(UserInterface.run_argv())
> 
> === modified file 'test/test_ui_kde.py'
> --- test/test_ui_kde.py	2013-08-30 10:40:19 +0000
> +++ test/test_ui_kde.py	2015-03-25 13:19:48 +0000
> @@ -1,5 +1,6 @@
> -'''KDE 4 Apport User Interface tests'''
> +'''Qt 5 Apport User Interface tests'''
>  
> +# Copyright (C) 2015 Harald Sitter <sitter@xxxxxxx>
>  # Copyright (C) 2012 Canonical Ltd.
>  # Author: Evan Dandrea <evan.dandrea@xxxxxxxxxxxxx>
>  #
> @@ -17,10 +18,8 @@
>  
>  from mock import patch
>  try:
> -    from PyQt4.QtCore import QTimer, QCoreApplication
> -    from PyQt4.QtGui import QTreeWidget
> -    from PyKDE4.kdecore import ki18n, KCmdLineArgs, KAboutData, KLocalizedString
> -    from PyKDE4.kdeui import KApplication
> +    from PyQt5.QtCore import QTimer, QCoreApplication
> +    from PyQt5.QtGui import QApplication, QTreeWidget, QIcon
>  except ImportError as e:
>      sys.stderr.write('SKIP: PyQt/PyKDE not available: %s\n' % str(e))
>      sys.exit(0)
> @@ -592,20 +591,9 @@
>          self.assertFalse(self.app.dialog.send_error_report.isVisible())
>          self.assertFalse(self.app.dialog.send_error_report.isChecked())
>  
> -appName = 'apport-kde'
> -catalog = 'apport'
> -programName = ki18n(b'Apport KDE')
> -version = '1.0'
> -description = ki18n(b'KDE 4 frontend tests for the apport')
> -license = KAboutData.License_GPL
> -copyright = ki18n(b'2012 Canonical Ltd.')
> -text = KLocalizedString()
> -homePage = 'https://wiki.ubuntu.com/AutomatedProblemReports'
> -bugEmail = 'kubuntu-devel@xxxxxxxxxxxxxxxx'
> -
> -aboutData = KAboutData(appName, catalog, programName, version, description,
> -                       license, copyright, text, homePage, bugEmail)
> -
> -KCmdLineArgs.init([''], aboutData)
> -app = KApplication()
> +app = QApplication(sys.argv)
> +app.applicationName = 'apport-kde'
> +app.applicationDisplayName = _('Apport')
> +app.windowIcon = QIcon.fromTheme('apport')
> +
>  unittest.main()
> 


-- 
https://code.launchpad.net/~kubuntu-packagers/apport/qt5/+merge/254082
Your team Apport upstream developers is requested to review the proposed merge of lp:~kubuntu-packagers/apport/qt5 into lp:apport.


References