Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-runtime-timezone-fix into lp:ubuntu-clock-app
Review: Needs Fixing Please also at least fix the spelling error for "appering" in the commit message. -- https://code.launchpad.net/~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-runtime-timezone-fix/+merge/270396 Your team Ubuntu Clock Developers is subscribed to branch lp:ubuntu-clock-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-runtime-timezone-fix into lp:ubuntu-clock-app
Please also at least fix the spelling error for "appering" in the commit message. -- https://code.launchpad.net/~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-runtime-timezone-fix/+merge/270396 Your team Ubuntu Clock Developers is subscribed to branch lp:ubuntu-clock-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/music-app/music-app-reduce-png-size into lp:music-app
The proposal to merge lp:~gang65/music-app/music-app-reduce-png-size into lp:music-app has been updated. Status: Approved => Merged For more details, see: https://code.launchpad.net/~gang65/music-app/music-app-reduce-png-size/+merge/270230 -- Your team Music App Developers is subscribed to branch lp:music-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/music-app/music-app-reduce-png-size into lp:music-app
The proposal to merge lp:~gang65/music-app/music-app-reduce-png-size into lp:music-app has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~gang65/music-app/music-app-reduce-png-size/+merge/270230 -- Your team Music App Developers is subscribed to branch lp:music-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/samba-ui-05 into lp:ubuntu-filemanager-app
The proposal to merge lp:~carlos-mazieri/ubuntu-filemanager-app/samba-ui-05 into lp:ubuntu-filemanager-app has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-ui-05/+merge/270339 -- Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/samba-ui-05 into lp:ubuntu-filemanager-app
Review: Approve -- https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-ui-05/+merge/270339 Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/samba-ui-04 into lp:ubuntu-filemanager-app
The proposal to merge lp:~carlos-mazieri/ubuntu-filemanager-app/samba-ui-04 into lp:ubuntu-filemanager-app has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-ui-04/+merge/270338 -- Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/samba-ui-04 into lp:ubuntu-filemanager-app
Review: Approve -- https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-ui-04/+merge/270338 Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/samba-ui-03 into lp:ubuntu-filemanager-app
The proposal to merge lp:~carlos-mazieri/ubuntu-filemanager-app/samba-ui-03 into lp:ubuntu-filemanager-app has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-ui-03/+merge/270337 -- Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/samba-ui-03 into lp:ubuntu-filemanager-app
Review: Approve -- https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-ui-03/+merge/270337 Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/samba-ui-02 into lp:ubuntu-filemanager-app
I agree with your comments and I will modify that according to that. Thanks. -- https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-ui-02/+merge/270335 Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/music-app/music-app-reduce-png-size into lp:music-app
Review: Approve This looks good to me now. Thanks for the optimizations and the patience, Bartosz! :) -- https://code.launchpad.net/~gang65/music-app/music-app-reduce-png-size/+merge/270230 Your team Music App Developers is subscribed to branch lp:music-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/samba-ui-02 into lp:ubuntu-filemanager-app
Review: Needs Fixing
Several inlined comments. The one about removing all keys but the new ones
introduced in this change is the one that needs attention, others are more
minor.
Diff comments:
> === modified file 'src/plugin/placesmodel/placesmodel.cpp'
> --- src/plugin/placesmodel/placesmodel.cpp2015-02-11 19:22:08 +
> +++ src/plugin/placesmodel/placesmodel.cpp2015-09-07 21:27:29 +
> @@ -27,12 +28,14 @@
>
> PlacesModel::PlacesModel(QObject *parent) :
> QAbstractListModel(parent)
> + , m_userSavedLocationsName("userSavedLocations")
There's no reason to have userSavedLocationsName nor userRemovedLocationsName
as object's instance variables. They can not be set per instance nor they can
be modified after object creation, nor is there any reason why the should be.
Perhaps better to make them constants like:
namespace {
const char *userSavedLocationsName = "userSavedLocationsName";
...
}
or something like that
> + , m_userRemovedLocationsName("userRemovedLocations")
> + , m_going_to_rescanMtab(false)
> {
> m_userMountLocation = "/media/" + qgetenv("USER");
> // For example /run/user/1000
> m_runtimeLocations =
> QStandardPaths::standardLocations(QStandardPaths::RuntimeLocation);
>
> -QStringList defaultLocations;
> // Set the storage location to a path that works well
> // with app isolation
> QString settingsLocation =
> @@ -40,29 +43,43 @@
> + "/" + QCoreApplication::applicationName() + "/" +
> "places.conf";
> m_settings = new QSettings(settingsLocation, QSettings::IniFormat, this);
>
> +m_userSavedLocations =
> m_settings->value(m_userSavedLocationsName).toStringList();
> +m_userRemovedLocations =
> m_settings->value(m_userRemovedLocationsName).toStringList();
> +
> +//remove old key "storedLocations" or others no longer used
> +QStringList settingsKeys = m_settings->allKeys();
> +foreach (const QString& key, settingsKeys) {
> + if (key != m_userSavedLocationsName && key !=
> m_userRemovedLocationsName) {
This is error prone. No one will remember this piece of code when adding new
key to the settings. Instead it should only remove "storedLocations" if that
is the deprecated piece of code. Preferrably a TODO to remove the block all
together at some reasonable point at time (one year from now or whatever?)
> + m_settings->remove(key);
> + }
> +}
> +
> // Prepopulate the model with the user locations
> // for the first time it's used
> -defaultLocations.append(locationHome());
> -defaultLocations.append(locationDocuments());
> -defaultLocations.append(locationDownloads());
> -defaultLocations.append(locationMusic());
> -defaultLocations.append(locationPictures());
> -defaultLocations.append(locationVideos());
> -// Add root also
> -defaultLocations.append("/");
> -
> -if (!m_settings->contains("storedLocations")) {
> -m_locations.append(defaultLocations);
> -} else {
> -m_locations = m_settings->value("storedLocations").toStringList();
> +addDefaultLocation(locationHome());
> +addDefaultLocation(locationDocuments());
> +addDefaultLocation(locationDownloads());
> +addDefaultLocation(locationMusic());
> +addDefaultLocation(locationPictures());
> +addDefaultLocation(locationVideos());
> +
> +//Network locations
> +addDefaultLocation(locationSamba());
> +
> +//mounted locations
> +addDefaultLocation("/");
> +initNewUserMountsWatcher();
> +rescanMtab();
> +
> +//other user saved locations
> +foreach (const QString& userLocation, m_userSavedLocations) {
> + addLocationWithoutStoring(userLocation);
> }
> +m_settings->sync();
>
> foreach (const QString &location, m_locations) {
> qDebug() << "Location: " << location;
> }
> -
> -initNewUserMountsWatcher();
> -rescanMtab();
> }
>
> PlacesModel::~PlacesModel() {
> @@ -250,18 +305,35 @@
> endRemoveRows();
> }
>
> +
> void PlacesModel::addLocation(const QString &location)
> -{
> -if (addLocationWithoutStoring(location)) {
> -// Store the location permanently
> -m_settings->setValue("storedLocations", m_locations);
> +{
> +bool sync_seettings = false;
sync_seettings -> sync_settings
> +//verify it the user had deleted it before and now is inserting it again
> +int indexRemoved = m_userRemovedLocations.indexOf(location);
> +if(indexRemoved > -1) {
Spaces: if (indexRemoved > -1) {
> +m_userRemovedLocations.removeAt(indexRemoved);
> +m_settings->setValue(m_userRemovedLocationsName,
> m_userRemovedLocations);
> +sync_seettings = true;
> +}
> +if (addLocationWithoutStoring(location)) {
> +// Store the location permanently if it is not default location
> +if (!isDefaultLocation(location) &&
> !m_userSavedLocations.contains(location))
> +
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mzanetti/reminders-app/fix-writeback-issue into lp:reminders-app
The proposal to merge lp:~mzanetti/reminders-app/fix-writeback-issue into lp:reminders-app has been updated. Status: Approved => Merged For more details, see: https://code.launchpad.net/~mzanetti/reminders-app/fix-writeback-issue/+merge/263848 -- Your team Ubuntu Notes app developers is subscribed to branch lp:reminders-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/samba-ui-01 into lp:ubuntu-filemanager-app
Review: Needs Fixing
Diff comments:
> === modified file 'src/plugin/folderlistmodel/dirmodel.cpp'
> --- src/plugin/folderlistmodel/dirmodel.cpp 2015-08-25 18:41:31 +
> +++ src/plugin/folderlistmodel/dirmodel.cpp 2015-09-07 21:20:53 +
> @@ -542,12 +542,20 @@
> }
>
> bool DirModel::allowAccess(const DirItemInfo &fi) const {
> -return allowAccess(fi.absoluteFilePath());
> +bool allowed = !mOnlyAllowedPaths; // !mOnlyAllowedPaths means any path
> is allowed
> +if (!allowed)
> +{
> +// for remote locations items are visible if them do not require
> authentication
> +allowed = mCurLocation->isRemote() ? !fi.needsAuthentication() :
> +
> isAllowedPath(fi.absoluteFilePath());
> +}
> +return allowed;
> }
>
> bool DirModel::allowAccess(const QString &absoluteFilePath) const {
> -return !mOnlyAllowedPaths || isAllowedPath(absoluteFilePath);
> -}
> +return !mOnlyAllowedPaths || mCurLocation->isRemote() ||
> isAllowedPath(absoluteFilePath);
This doesn't make sense to me. AllowAccess(absoluteFilePath) is supposed to say
if the given absolute file path is allowed access. Instead now it always allows
access if current location happens to be remote path, and doesn't in that case
take into account if the given path given in parameter is allowed access.
This function and allowAccess(const DirItemInfo &fi) should have the same
logic, they just receive their parameter in different representation. But it
must be said that after your refactorings it can be that either one of the
functions is not necessary anymore.
But if both exist they should either have same logic, preferably sharing the
code if possible, or the given function parameters and/or function name reflect
what's the real purpose here.
> +}// for remote locations access is allowed
> +
>
> void DirModel::onItemsAdded(const DirItemInfoList &newFiles)
> {
--
https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-ui-01/+merge/270334
Your team Ubuntu File Manager Developers is subscribed to branch
lp:ubuntu-filemanager-app.
--
Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers
Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers
More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mzanetti/reminders-app/fix-writeback-issue into lp:reminders-app
Review: Approve lgtm, thanks! -- https://code.launchpad.net/~mzanetti/reminders-app/fix-writeback-issue/+merge/263848 Your team Ubuntu Notes app developers is subscribed to branch lp:reminders-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mzanetti/reminders-app/fix-writeback-issue into lp:reminders-app
The proposal to merge lp:~mzanetti/reminders-app/fix-writeback-issue into lp:reminders-app has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~mzanetti/reminders-app/fix-writeback-issue/+merge/263848 -- Your team Ubuntu Notes app developers is subscribed to branch lp:reminders-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-runtime-timezone-fix into lp:ubuntu-clock-app
Review: Needs Fixing I haven't had the time to do a full code review yet, but I see it is missing manual tests for this bug 1493358 and bug 1491024. Please fix that. It should at most another 10 mins. -- https://code.launchpad.net/~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-runtime-timezone-fix/+merge/270396 Your team Ubuntu Clock Developers is subscribed to branch lp:ubuntu-clock-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mzanetti/reminders-app/fix-writeback-issue into lp:reminders-app
Review: Approve continuous-integration PASSED: Continuous integration, rev:469 http://91.189.93.70:8080/job/reminders-app-ci/778/ Executed test runs: SUCCESS: http://91.189.93.70:8080/job/reminders-app-vivid-amd64-ci/200 Click here to trigger a rebuild: http://91.189.93.70:8080/job/reminders-app-ci/778/rebuild -- https://code.launchpad.net/~mzanetti/reminders-app/fix-writeback-issue/+merge/263848 Your team Ubuntu Notes app developers is subscribed to branch lp:reminders-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-runtime-timezone-fix into lp:ubuntu-clock-app
Review: Approve continuous-integration PASSED: Continuous integration, rev:374 http://91.189.93.70:8080/job/ubuntu-clock-app-ci/829/ Executed test runs: SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/174 Click here to trigger a rebuild: http://91.189.93.70:8080/job/ubuntu-clock-app-ci/829/rebuild -- https://code.launchpad.net/~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-runtime-timezone-fix/+merge/270396 Your team Ubuntu Clock Developers is requested to review the proposed merge of lp:~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-runtime-timezone-fix into lp:ubuntu-clock-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~popey/ubuntu-filemanager-app/add-click-deps into lp:ubuntu-filemanager-app
It is hard to say something about the crash, if you have the 'libsmbclient' installed in your phone you can try this click package which contains 'Network' in Places to start navigating in Samba. https://www.dropbox.com/s/f6gxiqbav3th8bx/com.ubuntu.filemanager_0.4.latest_armhf.click?dl=0 Have you tested on Desktop? On 9/8/15, Alan Pope wrote: > Well spotted. We should disable the keyboard helpers (which try to > autocorrect when you type in a location) and that will stop the space > getting added, and also stop smb being 'corrected' to sob. > > I tried again and this time the file manager crashes. > > http://termbin.com/d8x0 > > The crash dump was uploaded, and processed. It's a secured page so I printed > it as a PDF, I hope that's readable. > > http://people.canonical.com/~alan/filemanager_crash.pdf > -- > https://code.launchpad.net/~popey/ubuntu-filemanager-app/add-click-deps/+merge/270287 > Your team Ubuntu File Manager Developers is requested to review the proposed > merge of lp:~popey/ubuntu-filemanager-app/add-click-deps into > lp:ubuntu-filemanager-app. > -- https://code.launchpad.net/~popey/ubuntu-filemanager-app/add-click-deps/+merge/270287 Your team Ubuntu File Manager Developers is requested to review the proposed merge of lp:~popey/ubuntu-filemanager-app/add-click-deps into lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/music-app/music-app-reduce-png-size into lp:music-app
Review: Approve continuous-integration PASSED: Continuous integration, rev:918 http://91.189.93.70:8080/job/music-app-ci/1378/ Executed test runs: SUCCESS: http://91.189.93.70:8080/job/music-app-vivid-amd64-ci/230 Click here to trigger a rebuild: http://91.189.93.70:8080/job/music-app-ci/1378/rebuild -- https://code.launchpad.net/~gang65/music-app/music-app-reduce-png-size/+merge/270230 Your team Music App Developers is subscribed to branch lp:music-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/music-app/music-app-reduce-png-size into lp:music-app
Thanks Victor and Andrew. Fix applied. -- https://code.launchpad.net/~gang65/music-app/music-app-reduce-png-size/+merge/270230 Your team Music App Developers is subscribed to branch lp:music-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-runtime-timezone-fix into lp:ubuntu-clock-app
Review: Approve continuous-integration PASSED: Continuous integration, rev:373 http://91.189.93.70:8080/job/ubuntu-clock-app-ci/828/ Executed test runs: SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/173 Click here to trigger a rebuild: http://91.189.93.70:8080/job/ubuntu-clock-app-ci/828/rebuild -- https://code.launchpad.net/~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-runtime-timezone-fix/+merge/270396 Your team Ubuntu Clock Developers is requested to review the proposed merge of lp:~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-runtime-timezone-fix into lp:ubuntu-clock-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/music-app/music-app-reduce-png-size into lp:music-app
I agree with Andrew. The framework bump was released in the release before the last [1]. 1 - http://pad.ubuntu.com/MusicAppReadMe -- https://code.launchpad.net/~gang65/music-app/music-app-reduce-png-size/+merge/270230 Your team Music App Developers is subscribed to branch lp:music-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-runtime-timezone-fix into lp:ubuntu-clock-app
Bartosz Kosiorek has proposed merging
lp:~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-runtime-timezone-fix into
lp:ubuntu-clock-app.
Commit message:
Fix stopwatch issue appering during changing timezone during runtime (LP:
#1493358)
Requested reviews:
Ubuntu Clock Developers (ubuntu-clock-dev)
Related bugs:
Bug #1493358 in Ubuntu Clock App: "Stopwatch is not working correctly after
change timezone during runtime"
https://bugs.launchpad.net/ubuntu-clock-app/+bug/1493358
For more details, see:
https://code.launchpad.net/~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-runtime-timezone-fix/+merge/270396
Fix stopwatch issue appering during changing timezone during runtime (LP:
#1493358)
--
Your team Ubuntu Clock Developers is requested to review the proposed merge of
lp:~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-runtime-timezone-fix into
lp:ubuntu-clock-app.
=== modified file 'app/MainPage.qml'
--- app/MainPage.qml 2015-08-20 16:03:35 +
+++ app/MainPage.qml 2015-09-08 12:52:12 +
@@ -63,9 +63,8 @@
Settings {
id: stopwatchState
category: "Stopwatch"
-property alias startTime: stopwatchPage.startTime
property alias running: stopwatchPage.running
-property alias oldDiff: stopwatchPage.oldDiff
+property alias oldDiff: stopwatchPage.previousLapsTime
}
VisualItemModel {
=== modified file 'app/stopwatch/StopwatchPage.qml'
--- app/stopwatch/StopwatchPage.qml 2015-09-02 13:15:50 +
+++ app/stopwatch/StopwatchPage.qml 2015-09-08 12:52:12 +
@@ -24,51 +24,43 @@
id: _stopwatchPage
objectName: "stopwatchPage"
-property date startTime: getUTCDate()
-property date snapshot: startTime
+
+StopwatchFormatTime {
+id: stopwatchFormatTime
+}
+
+//property date startTime: getUTCDate()
+//property date snapshot: startTime
property bool running: false
-property int timeDiff: snapshot - startTime
-property int oldDiff: 0
+property int lapTime: 0
+property int previousLapsTime: 0
-property int totalTimeDiff: timeDiff + oldDiff
+property int totalTime: lapTime + previousLapsTime
Component.onCompleted: {
console.log("[LOG]: Stopwatch Page Loaded")
}
-function getUTCDate() {
-var localDate = new Date()
-return new Date(localDate.getUTCFullYear(),
-localDate.getUTCMonth(),
-localDate.getUTCDate(),
-localDate.getUTCHours(),
-localDate.getUTCMinutes(),
-localDate.getUTCSeconds(),
-localDate.getUTCMilliseconds())
-}
-
function start() {
-startTime = getUTCDate()
-snapshot = startTime
+if (lapTime === 0) {
+lapHistory.startStopwatch();
+}
running = true
}
function stop() {
-oldDiff += timeDiff
-startTime = getUTCDate()
-snapshot = startTime
running = false
}
function update() {
-snapshot = getUTCDate()
+lapTime = lapHistory.updateStopwatch();
}
function clear() {
-oldDiff = 0
-startTime = getUTCDate()
-snapshot = startTime
+running = false
+lapTime = 0
+previousLapsTime = 0
lapHistory.clear()
}
@@ -86,7 +78,7 @@
id: stopwatch
objectName: "stopwatch"
-milliseconds: _stopwatchPage.totalTimeDiff
+milliseconds: _stopwatchPage.totalTime
anchors {
top: parent.top
@@ -109,9 +101,9 @@
Button {
id: stopButton
-width: oldDiff !== 0 || running ? (parent.width - parent.spacing) / 2 : parent.width
+width: previousLapsTime !== 0 || running ? (parent.width - parent.spacing) / 2 : parent.width
color: !_stopwatchPage.running ? UbuntuColors.green : UbuntuColors.red
-text: _stopwatchPage.running ? i18n.tr("Stop") : (oldDiff === 0 ? i18n.tr("Start") : i18n.tr("Resume"))
+text: _stopwatchPage.running ? i18n.tr("Stop") : (previousLapsTime === 0 ? i18n.tr("Start") : i18n.tr("Resume"))
onClicked: {
if (_stopwatchPage.running) {
_stopwatchPage.stop()
@@ -129,13 +121,13 @@
Button {
id: lapButton
text: _stopwatchPage.running ? i18n.tr("Lap") : i18n.tr("Clear")
-width: oldDiff !== 0 || running ? (parent.width - parent.spacing) / 2 : 0
+width: previousLapsTime !== 0 || running ? (parent.width - parent.spacing) / 2 : 0
strokeColor: UbuntuColors.lightGrey
-visible: oldDiff !== 0 || running
+visible: previousLapsTime !== 0 || running
onClicked: {
if (_stopwatchPage.running) {
_stopwatchPage.update()
-lapHistory.addLap(_stopwatchPage.totalTimeDiff)
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/music-app/music-app-reduce-png-size into lp:music-app
I was expecting the debian/changelog diff to look like this [0], as 2.2 was released, then 2.2ubuntu1 and now 2.2ubuntu2 is the unreleased WIP version. Right Victor? 0 - http://pastebin.ubuntu.com/12313538/ -- https://code.launchpad.net/~gang65/music-app/music-app-reduce-png-size/+merge/270230 Your team Music App Developers is subscribed to branch lp:music-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/music-app/music-app-reduce-png-size into lp:music-app
Review: Needs Information According to last changelog, from: https://uappexplorer.com/app/com.ubuntu.music In last release we delivered: * Always fit the cover art to vertically fill the BlurredBackground. * Allow arrow keys to control the initial walkthrough. Fixes: http://pad.lv/1482760 * Force playlist tracks to sync. Fixes: https://pad.lv/1483962 * Add support for a content-hub source in both single and multiple selection modes. Fixes: http://pad.lv/1357324 * Translation updates And of course migration to 15.04. I think there was an mistake in changelog, that's why I fixed that. Please correct me if I'm wrong. -- https://code.launchpad.net/~gang65/music-app/music-app-reduce-png-size/+merge/270230 Your team Music App Developers is subscribed to branch lp:music-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/music-app/music-app-reduce-png-size into lp:music-app
Review: Needs Information There's still some odd issues with the changelog if you look at the diff. Previously we had released 2.2ubuntu1 and now you're unreleasing it. Could you revert the other changelog modifications and only add an unreleased section for 2.2ubuntu2? Sorry I suggested 2.2ubuntu1, I was using the already broken changelog as an example. -- https://code.launchpad.net/~gang65/music-app/music-app-reduce-png-size/+merge/270230 Your team Music App Developers is subscribed to branch lp:music-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~popey/ubuntu-filemanager-app/add-click-deps into lp:ubuntu-filemanager-app
Well spotted. We should disable the keyboard helpers (which try to autocorrect when you type in a location) and that will stop the space getting added, and also stop smb being 'corrected' to sob. I tried again and this time the file manager crashes. http://termbin.com/d8x0 The crash dump was uploaded, and processed. It's a secured page so I printed it as a PDF, I hope that's readable. http://people.canonical.com/~alan/filemanager_crash.pdf -- https://code.launchpad.net/~popey/ubuntu-filemanager-app/add-click-deps/+merge/270287 Your team Ubuntu File Manager Developers is requested to review the proposed merge of lp:~popey/ubuntu-filemanager-app/add-click-deps into lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~popey/ubuntu-filemanager-app/add-click-deps into lp:ubuntu-filemanager-app
Looking at the log you posted, it looks like there is an extra space after "smb": void DirModel::setPath(const QString&, const QString&, const QString&, bool) DirModel_QML_137(0x12ff010) path or url may not exist or cannot be read: "smb ://192.168.1.3/" The correct url must be "smb://192.168.1.3". BTW, I have had some hard time trying to type "smb" urls on the device. On 9/8/15, Alan Pope wrote: > Just tried this click on my "retail" bq e4.5 (so it's still read only - not > had any debian packages installed - is as a customer would find it). Not > able to browse to my NAS. > > http://termbin.com/vdt4 > > Is there a "known good" config that this should connect to? > -- > https://code.launchpad.net/~popey/ubuntu-filemanager-app/add-click-deps/+merge/270287 > Your team Ubuntu File Manager Developers is requested to review the proposed > merge of lp:~popey/ubuntu-filemanager-app/add-click-deps into > lp:ubuntu-filemanager-app. > -- https://code.launchpad.net/~popey/ubuntu-filemanager-app/add-click-deps/+merge/270287 Your team Ubuntu File Manager Developers is requested to review the proposed merge of lp:~popey/ubuntu-filemanager-app/add-click-deps into lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/music-app/music-app-reduce-png-size into lp:music-app
Review: Approve continuous-integration PASSED: Continuous integration, rev:917 http://91.189.93.70:8080/job/music-app-ci/1377/ Executed test runs: SUCCESS: http://91.189.93.70:8080/job/music-app-vivid-amd64-ci/229 Click here to trigger a rebuild: http://91.189.93.70:8080/job/music-app-ci/1377/rebuild -- https://code.launchpad.net/~gang65/music-app/music-app-reduce-png-size/+merge/270230 Your team Music App Developers is subscribed to branch lp:music-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~popey/ubuntu-filemanager-app/add-click-deps into lp:ubuntu-filemanager-app
Just tried this click on my "retail" bq e4.5 (so it's still read only - not had any debian packages installed - is as a customer would find it). Not able to browse to my NAS. http://termbin.com/vdt4 Is there a "known good" config that this should connect to? -- https://code.launchpad.net/~popey/ubuntu-filemanager-app/add-click-deps/+merge/270287 Your team Ubuntu File Manager Developers is requested to review the proposed merge of lp:~popey/ubuntu-filemanager-app/add-click-deps into lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~gang65/music-app/music-app-reduce-png-size into lp:music-app
Thanks Victor for explanation. I didn't know that minor version is increasing only during some major changes. I reverted these changes. -- https://code.launchpad.net/~gang65/music-app/music-app-reduce-png-size/+merge/270230 Your team Music App Developers is subscribed to branch lp:music-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
