ubuntu-touch-coreapps-reviewers team mailing list archive

[Arto Jalkanen <ajalkane@xxxxxxxxx>]

Review: Approve

Some comments to consider for fixing, but approving:

39: Spelling mistake: indexOfColonAndSlashe -> indexOfColonAndSlashes

42: Instead of hardcoding the location of startOf "://" in DiskRootUrl here with midRef(0,5) it would be better to use a constant defined in LocationUrl, or have a static method to determine this in LocationUrl. It can be error prone to distribute this logic across classes.


Diff comments:

> 
> === modified file 'src/plugin/folderlistmodel/disk/disklocation.cpp'
> --- src/plugin/folderlistmodel/disk/disklocation.cpp	2015-03-01 15:32:42 +0000
> +++ src/plugin/folderlistmodel/disk/disklocation.cpp	2015-07-18 22:03:45 +0000
> @@ -168,3 +175,37 @@
>      return new DirListWorker(urlPath,filter,isRecursive);
>  }
>  
> +
> +QString DiskLocation::urlBelongsToLocation(const QString &urlPath, int indexOfColonAndSlashe)

Spelling mistake: indexOfColonAndSlashe -> indexOfColonAndSlashes

> +{
> +    QString ret;
> +    if (urlPath.startsWith(LocationUrl::DiskRootURL.midRef(0,5)))

Instead of hardcoding the location of startOf "://" in DiskRootUrl here with midRef(0,5) it would be better to use a constant defined in LocationUrl, or have a static method to determine this in LocationUrl. It can be error prone to distribute this logic across classes.

> +    {
> +        ret  = QDir::rootPath() + DirItemInfo::removeExtraSlashes(urlPath, indexOfColonAndSlashe+1);
> +    }
> +    return ret;
> +}
> +
> +
> +LocationItemDirIterator *
> +DiskLocation::newDirIterator(const QString &path,
> +                             QDir::Filters filters,
> +                             QDirIterator::IteratorFlags flags)
> +{
> +    return  new DiskLocationItemDirIterator(path, filters, flags);
> +}
> +
> +
> +bool DiskLocation::isThereDiskSpace(const QString &pathname, qint64 requiredSize)
> +{
> +    bool ret = true;
> +#if defined(Q_OS_UNIX)
> +    struct statvfs  vfs;
> +    if ( ::statvfs( QFile::encodeName(pathname).constData(), &vfs) == 0 )
> +    {
> +        qint64 free =  vfs.f_bsize * vfs.f_bfree;
> +        ret = free > requiredSize;
> +    }
> +#endif
> +   return ret;
> +}


-- 
https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-actions-08/+merge/265199
Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app.