Code review comment for lp:~emailgirishrawat/ubuntu-docviewer-app/READMEs

Revision history for this message
Stefano Verzegnassi (verzegnassi-stefano) wrote :

Hi,

Thanks for this big improvement!

I had a look at your MP and overall it looks very good.
However, some minor change is required in order to reflect the current status of the project.

* Some of the Autopilot tests are currently broken.
  It may be worth to point that out in the 'README-Mergeproposal.md' file, explaining that they aren't strictly required at the moment. The same goes for 'README-Autopilot.md' file.

* In the 'README-Developers.md' file, we mention that LibreOffice 5.0 is not available in the archive. This is no longer true, since it's available in Ubuntu 15.10 archives.
  For earlier release, we may want to refer to the following PPA, which includes only stable releases:
     ppa:libreoffice/libreoffice-5-0

* Still in the same file, in the 'Compiling" section, it may be useful to specify that a working internet connection is required in order to build the project, since a number of Debian packages are downloaded when the project is loaded the first time through (e.g.) QtCreator.

* Still a small correction to 'README-Mergeproposal'.
  The line:
    "Does the MP change the UI? If Yes, has it been approved by design?"
  does not reflect our current workflow, since a large part of the UI hasn't been approved by the design team (it has been approved by the devs instead).
  Please replace that line with:
    "Does the MP change the UI? If Yes, has it been approved by design, or discussed with some of the DocViewer developers?"

Thanks again!

review: Needs Fixing

« Back to merge proposal