[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~michael-sheldon/ubuntu-terminal-app/fix-1628781 into lp:ubuntu-terminal-app

The proposal to merge lp:~michael-sheldon/ubuntu-terminal-app/fix-1628781 into lp:ubuntu-terminal-app has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~michael-sheldon/ubuntu-terminal-app/fix-1628781/+merge/307172 -- Your team Ubuntu

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~michael-sheldon/ubuntu-terminal-app/fix-1628781 into lp:ubuntu-terminal-app

Review: Approve Thanks from someone who got bitten by this yesterday :) -- https://code.launchpad.net/~michael-sheldon/ubuntu-terminal-app/fix-1628781/+merge/307172 Your team Ubuntu Terminal Developers is subscribed to branch lp:ubuntu-terminal-app. -- Mailing list:

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~michael-sheldon/ubuntu-terminal-app/packaging-fixes into lp:ubuntu-terminal-app

Review: Approve Approved as per changes discussed on bug #1625074 -- https://code.launchpad.net/~michael-sheldon/ubuntu-terminal-app/packaging-fixes/+merge/306252 Your team Ubuntu Terminal Developers is subscribed to branch lp:ubuntu-terminal-app. -- Mailing list:

[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~popey/ubuntu-terminal-app/add-snapcraft-config into lp:ubuntu-terminal-app

The proposal to merge lp:~popey/ubuntu-terminal-app/add-snapcraft-config into lp:ubuntu-terminal-app has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~popey/ubuntu-terminal-app/add-snapcraft-config/+merge/305206 -- Your team Ubuntu

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~popey/ubuntu-terminal-app/add-snapcraft-config into lp:ubuntu-terminal-app

Review: Approve Looks good to me, thanks! I think we can merge it as it is. CLICK_MODE=on is not necessary and installs extra files [1], but it seems the snap works, so we might as well leave it as it is for a first pass. [1]

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~popey/ubuntu-terminal-app/add-snapcraft-config into lp:ubuntu-terminal-app

Replied inline again, thanks! Diff comments: > > === modified file 'src/plugin/qmltermwidget/CMakeLists.txt' > --- src/plugin/qmltermwidget/CMakeLists.txt 2014-11-15 14:39:05 + > +++ src/plugin/qmltermwidget/CMakeLists.txt 2016-09-08 11:44:05 + > @@ -70,6 +70,7 @@ > > # Copy the

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~popey/ubuntu-terminal-app/add-snapcraft-config into lp:ubuntu-terminal-app

I've added another comment, but I also just noticed that this branch attempts to fix the same thing, but adding the CMake rule somewhere else: https://code.launchpad.net/~larryprice/ubuntu-terminal-app/fix-cmake-xenial/+merge/297394 Diff comments: > > === modified file

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~popey/ubuntu-terminal-app/add-snapcraft-config into lp:ubuntu-terminal-app

Review: Needs Information Nice work! I've added some comments. Right now, if I understand it correctly, the terminal will only run on Unity 7, whereas we'd like to get it running on Unity 8. I'm just adding it as a reminder, which shouldn't block this MP to get approved after reviewing the

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~nikwen/ubuntu-calendar-app/fix-standalone-month-name-i18n into lp:ubuntu-calendar-app

Thanks Niklas, and good work everyone! On Wed, Apr 6, 2016 at 10:38 PM, Niklas Wenzel wrote: > Finished merging. Thank you for your patience, Nekhelesh! :) > -- > > https://code.launchpad.net/~nikwen/ubuntu-calendar-app/fix-standalone-month-name-i18n/+merge/279509 >

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mardy/reminders-app/new-api into lp:reminders-app

Review: Needs Information Bug #1512667 seems to have been fixed right now. Are we in a position to land this branch? -- https://code.launchpad.net/~mardy/reminders-app/new-api/+merge/271505 Your team Ubuntu Notes app developers is subscribed to branch lp:reminders-app. -- Mailing list:

[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~dpm/ubuntu-calculator-app/fix-deb-build-bug-1551848 into lp:ubuntu-calculator-app

David Planella has proposed merging lp:~dpm/ubuntu-calculator-app/fix-deb-build-bug-1551848 into lp:ubuntu-calculator-app. Commit message: Fix .deb package failure to build Requested reviews: Ubuntu Calculator Developers (ubuntu-calculator-dev) Related bugs: Bug #1551848 in Ubuntu

[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~dpm/sudoku-app/rm-pot into lp:sudoku-app

David Planella has proposed merging lp:~dpm/sudoku-app/rm-pot into lp:sudoku-app. Requested reviews: Sudoku Touch developers (sudoku-touch-dev) For more details, see: https://code.launchpad.net/~dpm/sudoku-app/rm-pot/+merge/285397 The additional sudoku-app.pot file, dating from 2014

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~nikwen/ubuntu-calendar-app/fix-standalone-month-name-i18n into lp:ubuntu-calendar-app

Review: Needs Fixing I've just double-checked this again, and I can confirm that the qsTr() call is not required. i18n.tr() (gettext) already takes care of the format string, it is not necessary to process it again with qsTr(). This can easily be tested with a simple QML app that includes the

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mcintire-evan/ubuntu-terminal-app/improve_readmes into lp:ubuntu-terminal-app

Review: Needs Fixing Thanks Evan! I don't know at which point the original header format was changed, but had I seen it, I would have objected so that it would have not generated extra work, and we would not have landed in the situation where we are in. That said, we're always really thankful

[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~dpm/reminders-app/fix-deb-build into lp:reminders-app

David Planella has proposed merging lp:~dpm/reminders-app/fix-deb-build into lp:reminders-app. Requested reviews: Jenkins Bot (ubuntu-core-apps-jenkins-bot): continuous-integration Ubuntu Notes app developers (notes-app-dev) Related bugs: Bug #1539482 in Ubuntu Notes app: "Daily b

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mcintire-evan/ubuntu-terminal-app/desktop-dimensions into lp:ubuntu-terminal-app

Thanks a lot Evan! I think this effectively fixes the bug, but on a second iteration, I wonder whether we could have dynamic conversion of gu to column width/height, so that we could set the default width to e.g. 80 columns by 25 rows and it would result in the same column/row size on all

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mcintire-evan/ubuntu-terminal-app/improve_readmes into lp:ubuntu-terminal-app

Thanks a lot for the improvements! As the person who originally wrote the READMEs, please note that on the "Formatted READMEs with MarkDown", they are already in valid markdown format. I would suggest not changing the ## vs == header [1] formatting, as it's a subjective preference, and if we

[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~dpm/reminders-app/fix-deb-build into lp:reminders-app

The proposal to merge lp:~dpm/reminders-app/fix-deb-build into lp:reminders-app has been updated. Commit Message changed to: Fix missing Oxide build dependency For more details, see: https://code.launchpad.net/~dpm/reminders-app/fix-deb-build/+merge/284423 -- Your team Ubuntu Notes app

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mcintire-evan/ubuntu-terminal-app/disable-paste into lp:ubuntu-terminal-app

It does, although the individual commits are still there and can be seen under the main merge commit with tools such as 'bzr qlog' ('sudo apt install qbzr' to use it). -- https://code.launchpad.net/~mcintire-evan/ubuntu-terminal-app/disable-paste/+merge/283244 Your team Ubuntu Terminal

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~nikwen/ubuntu-calendar-app/fix-standalone-month-name-i18n into lp:ubuntu-calendar-app

Review: Needs Information Why do you need to call qsTr()? IIRC, it should work with i18n.tr() only already. qsTr(i18n.tr("%1 %2")).arg(Qt.locale().standaloneMonthName(currentMonth.getMonth(), Locale.LongFormat)).arg(currentMonth.getFullYear()) --

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~nikwen/ubuntu-calendar-app/fix-standalone-month-name-i18n into lp:ubuntu-calendar-app

Review: Needs Information Sorry, to be more specific: - Qt.locale() should choose the right locale for you already for the standaloneMonthName, without needing qsTr() - Does currentMonth.getFullYear() not need to be localized as well? --

[Ubuntu-touch-coreapps-reviewers] [Blueprint libreoffice-docviewer-integration] Libreoffice Docviewer Integration blueprint

Blueprint changed by David Planella: Work items changed: Work items for 2015-07-02: [bjoern-michaelsen] Investigate how the LOK-based gnome docviewer detects page breaks: DONE [bjoern-michaelsen] Study the API and report on capabilities that would be applicable to the LO viewer

[Ubuntu-touch-coreapps-reviewers] [Blueprint libreoffice-docviewer-integration] Libreoffice Docviewer Integration blueprint

Blueprint changed by David Planella: Work items changed: Work items for 2015-07-02: [bjoern-michaelsen] Investigate how the LOK-based gnome docviewer detects page breaks: DONE [bjoern-michaelsen] Study the API and report on capabilities that would be applicable to the LO viewer

[Ubuntu-touch-coreapps-reviewers] [Blueprint libreoffice-docviewer-integration] Libreoffice Docviewer Integration blueprint

Blueprint changed by David Planella: Work items changed: Work items for 2015-07-02: [bjoern-michaelsen] Investigate how the LOK-based gnome docviewer detects page breaks: DONE [bjoern-michaelsen] Study the API and report on capabilities that would be applicable to the LO viewer

[Ubuntu-touch-coreapps-reviewers] [Blueprint libreoffice-docviewer-integration] Libreoffice Docviewer Integration blueprint

Blueprint changed by David Planella: Work items changed: Work items for 2015-07-02: [bjoern-michaelsen] Investigate how the LOK-based gnome docviewer detects page breaks: DONE [bjoern-michaelsen] Study the API and report on capabilities that would be applicable to the LO viewer

[Ubuntu-touch-coreapps-reviewers] [Blueprint libreoffice-docviewer-integration] Libreoffice Docviewer Integration blueprint

Blueprint changed by David Planella: Whiteboard changed: Scratch pad at http://pad.ubuntu.com/loviewer + + Questions for 14 Aug 2015 (Stefano) + + * Status of a convergent UI/UX design for DocViewer. + Since our ultimate target is to get a DocViewer version, suitable for both PCs and devices

[Ubuntu-touch-coreapps-reviewers] [Blueprint libreoffice-docviewer-integration] Libreoffice Docviewer Integration blueprint

Blueprint changed by David Planella: Work items changed: Work items for 2015-07-02: [bjoern-michaelsen] Investigate how the LOK-based gnome docviewer detects page breaks: DONE [bjoern-michaelsen] Study the API and report on capabilities that would be applicable to the LO viewer

[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~dpm/ubuntu-filemanager-app/rename-pam-package into lp:ubuntu-filemanager-app

The proposal to merge lp:~dpm/ubuntu-filemanager-app/rename-pam-package into lp:ubuntu-filemanager-app has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~dpm/ubuntu-filemanager-app/rename-pam-package/+merge/263162 -- Your team Ubuntu File

[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~popey/ubuntu-docviewer-app/add-authors into lp:ubuntu-docviewer-app

The proposal to merge lp:~popey/ubuntu-docviewer-app/add-authors into lp:ubuntu-docviewer-app has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~popey/ubuntu-docviewer-app/add-authors/+merge/263925 -- Your team Ubuntu Document Viewer

[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~dpm/ubuntu-terminal-app/bump-deb-version into lp:ubuntu-terminal-app

David Planella has proposed merging lp:~dpm/ubuntu-terminal-app/bump-deb-version into lp:ubuntu-terminal-app. Commit message: Bumped Debian changelog version Requested reviews: Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot): continuous-integration Ubuntu Terminal Developers

[Ubuntu-touch-coreapps-reviewers] [Bug 1469283] Re: Naming conflict with Terminal PAM plugin on the core apps PPA

** Changed in: ubuntu-filemanager-app Status: New = Triaged ** Changed in: ubuntu-filemanager-app Importance: Undecided = Low -- You received this bug notification because you are a member of Ubuntu File Manager Developers, which is subscribed to Ubuntu File Manager App. Matching

[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/ubuntu-calculator-app/ubuntu-calculator-app-desktop-install-fix into lp:ubuntu-calculator-app

The proposal to merge lp:~gang65/ubuntu-calculator-app/ubuntu-calculator-app-desktop-install-fix into lp:ubuntu-calculator-app has been updated. Status: Needs review = Approved For more details, see:

[Ubuntu-touch-coreapps-reviewers] [Bug 1469283] Re: Naming conflict with Terminal PAM plugin on the core apps PPA

** Branch linked: lp:~dpm/ubuntu-filemanager-app/rename-pam-package -- You received this bug notification because you are a member of Ubuntu File Manager Developers, which is subscribed to Ubuntu File Manager App. Matching subscriptions: File Manager App Bugmail

[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~dpm/ubuntu-filemanager-app/rename-pam-package into lp:ubuntu-filemanager-app

David Planella has proposed merging lp:~dpm/ubuntu-filemanager-app/rename-pam-package into lp:ubuntu-filemanager-app. Requested reviews: Ubuntu File Manager Developers (ubuntu-filemanager-dev) Related bugs: Bug #1469283 in Ubuntu File Manager App: Naming conflict with Terminal PAM plugin

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/ubuntu-calculator-app/ubuntu-calculator-app-desktop-install-fix into lp:ubuntu-calculator-app

Review: Approve Added an inline comment to the debian/changelog, but approving nevertheless. Good work! Diff comments: === modified file 'CMakeLists.txt' --- CMakeLists.txt2015-04-14 16:42:56 + +++ CMakeLists.txt2015-06-25 20:39:01 + @@ -31,18 +31,17 @@ set(APP_HARDCODE

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/ubuntu-calculator-app/ubuntu-calculator-app-desktop-install-fix into lp:ubuntu-calculator-app

Review: Needs Fixing Just a few additional comments on debian/changelog and the launcher script. Looking good now, though! Diff comments: === modified file 'CMakeLists.txt' --- CMakeLists.txt2015-04-14 16:42:56 + +++ CMakeLists.txt2015-06-24 20:32:08 + @@ -27,22 +27,21 @@

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons into lp:ubuntu-docviewer-app

Did this branch ever land? It's marked as Approved instead of Merged, so it appears in the list of branches to review. If it landed, could it be marked as Merged, or alternatively as Rejected? Thanks! --

[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/ubuntu-docviewer-app/ubuntu-docviewer-app-icon-fix into lp:ubuntu-docviewer-app

The proposal to merge lp:~gang65/ubuntu-docviewer-app/ubuntu-docviewer-app-icon-fix into lp:ubuntu-docviewer-app has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~gang65/ubuntu-docviewer-app/ubuntu-docviewer-app-icon-fix/+merge/262911 --

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/ubuntu-docviewer-app/ubuntu-docviewer-app-icon-fix into lp:ubuntu-docviewer-app

Review: Approve Looks good, and after testing it, works well too. Thanks! Just a couple of notes that should not block this from landing: 1) On debian/changelog, you can use the (LP: #bug number) syntax to mark bugs as closed, instead of a URL. See [1] for details. 2) While building the

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/ubuntu-calculator-app/ubuntu-calculator-app-desktop-install-fix into lp:ubuntu-calculator-app

Added my replies inline, thanks! Diff comments: === modified file 'CMakeLists.txt' --- CMakeLists.txt2015-04-14 16:42:56 + +++ CMakeLists.txt2015-06-23 23:33:27 + @@ -27,13 +27,12 @@ include(GNUInstallDirs) -set(APP_NAMEubuntu-calculator-app)

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/ubuntu-calculator-app/ubuntu-calculator-app-desktop-install-fix into lp:ubuntu-calculator-app

Review: Needs Fixing Hi Bartosz, Thanks so much for looking into this! As you asked me for a review, I'll try to be thorough: 1. On the commit message for r199, there is no need to duplicate the Debian changelog on the commit message itself. Commit messages should be concise and to the point

[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~ubuntu-docviewer-dev/ubuntu-docviewer-app/lo-viewer into lp:ubuntu-docviewer-app

David Planella has proposed merging lp:~ubuntu-docviewer-dev/ubuntu-docviewer-app/lo-viewer into lp:ubuntu-docviewer-app. Requested reviews: Ubuntu Document Viewer Developers (ubuntu-docviewer-dev) For more details, see: https://code.launchpad.net/~ubuntu-docviewer-dev/ubuntu-docviewer-app

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~ricsipontaz/ubuntu-calendar-app/fix-for-bug-1428501 into lp:ubuntu-calendar-app

Review: Needs Fixing Thanks for the fix! Looks good to me, I'd just suggest to add a translator comment here. (See inline comment) Diff comments: === modified file 'TimeLineHeader.qml' --- TimeLineHeader.qml2015-02-26 16:44:43 + +++ TimeLineHeader.qml2015-03-05 08:07:05

[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~ricsipontaz/ubuntu-calendar-app/fix-for-bug-1428501 into lp:ubuntu-calendar-app

The proposal to merge lp:~ricsipontaz/ubuntu-calendar-app/fix-for-bug-1428501 into lp:ubuntu-calendar-app has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~ricsipontaz/ubuntu-calendar-app/fix-for-bug-1428501/+merge/251878 -- Your team

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~ricsipontaz/ubuntu-calendar-app/fix-for-bug-1428501 into lp:ubuntu-calendar-app

Review: Approve Looks good to me, thanks! -- https://code.launchpad.net/~ricsipontaz/ubuntu-calendar-app/fix-for-bug-1428501/+merge/251878 Your team Ubuntu Calendar Developers is subscribed to branch lp:ubuntu-calendar-app. -- Mailing list:

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~ricsipontaz/ubuntu-calendar-app/fix-for-bug-1428501 into lp:ubuntu-calendar-app

Review: Needs Fixing Thanks Richard, that's all that needs to be done. I.e. applying the fix, committing and pushing it. Then when everything is fine, a core app dev will approve the merge proposal, automated tests will be run against it and when they pass it will be merged to trunk. That

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~dholbach/help-app/1434210 into lp:help-app

Review: Approve LGTM now, thanks! Just one typo on the HACKING doc `make appl` - `make app` -- https://code.launchpad.net/~dholbach/help-app/1434210/+merge/254706 Your team Ubuntu Help app developers is subscribed to branch lp:help-app. -- Mailing list:

[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~dholbach/help-app/1434210 into lp:help-app

The proposal to merge lp:~dholbach/help-app/1434210 into lp:help-app has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~dholbach/help-app/1434210/+merge/254706 -- Your team Ubuntu Help app developers is subscribed to branch lp:help-app. --

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~nik90/ubuntu-clock-app/fix-translation-plural-forms into lp:ubuntu-clock-app

Review: Approve Looks good to me, just added two notes. Diff comments: === modified file 'app/alarm/AlarmSettingsPage.qml' --- app/alarm/AlarmSettingsPage.qml 2014-10-16 19:03:51 + +++ app/alarm/AlarmSettingsPage.qml 2015-03-12 23:25:02 + @@ -50,10 +50,10 @@

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~nik90/ubuntu-clock-app/predefined-world-city-translation-fix into lp:ubuntu-clock-app

Review: Approve I believe we don't need GETTEXT_LOCALEDIR unless we're explicitly using it somewhere. IIRC we use LOCALEDIR to install the .mo files in the right locations in clock, which is probably defined here:

Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~verzegnassi-stefano/ubuntu-docviewer-app/fix-translation-issues into lp:ubuntu-docviewer-app

Looks good to me, just added one comment. Not sure if it can be worked around? Diff comments: === modified file 'po/com.ubuntu.docviewer.pot' --- po/com.ubuntu.docviewer.pot 2015-03-04 12:28:46 + +++ po/com.ubuntu.docviewer.pot 2015-03-04 17:51:36 + @@ -8,7 +8,7 @@