> On Aug. 2, 2015, 12:38 p.m., Kai Uwe Broulik wrote:
> > applets/diskquota/plugin/DiskQuota.h, lines 111-113
> > <https://git.reviewboard.kde.org/r/124589/diff/2/?file=389327#file389327line111>
> >
> >     You could initialize these directly in the header file.
> >     
> >     bool m_quotaInstalled = true (or, well, false)
> >     etc

fixed.


> On Aug. 2, 2015, 12:38 p.m., Kai Uwe Broulik wrote:
> > applets/diskquota/plugin/QuotaListModel.h, line 36
> > <https://git.reviewboard.kde.org/r/124589/diff/2/?file=389331#file389331line36>
> >
> >     Aren't these protected?

No, they are public. Only begin/endInsertRows(), and begin/endRemoveRows() are 
protected.


> On Aug. 2, 2015, 12:38 p.m., Kai Uwe Broulik wrote:
> > applets/diskquota/package/contents/ui/main.qml, line 31
> > <https://git.reviewboard.kde.org/r/124589/diff/2/?file=389325#file389325line31>
> >
> >     I would prefer if that was a proper switch/if, if there's more than one 
> > condition with the ternary operator, especially with long enums, it gets 
> > pretty ugly
> >     
> >     Plasmoid.status: {
> >         switch (diskQuota.status) {
> >         case DiskQuota.ActiveStatus:
> >             return PlasmaCore.Types.ActiveStatus
> >         …
> >     }

I agree, fixed.


> On Aug. 2, 2015, 12:38 p.m., Kai Uwe Broulik wrote:
> > applets/diskquota/plugin/QuotaListModel.cpp, line 180
> > <https://git.reviewboard.kde.org/r/124589/diff/2/?file=389332#file389332line180>
> >
> >     Now I see why you need setData
> >     
> >     Imho a QStandardItemModel would have been sufficient for all of this :)

Maybe, the difference is not that large (I have a QStandardItem* instance still 
for each quota entry).


> On Aug. 2, 2015, 12:38 p.m., Kai Uwe Broulik wrote:
> > applets/diskquota/package/contents/ui/main.qml, line 67
> > <https://git.reviewboard.kde.org/r/124589/diff/2/?file=389325#file389325line67>
> >
> >     Why not just i18n("Quota tool not found.\n\nPlease install 'quota'.") ?

Initially, I wanted to have the text "Quota tool not found. Please install 
'quota' or contact your system administrator." I wanted to use wrapMode: 
Text.WordWrap, but as soon as the text is too long, plasmawindowed essentially 
freezes. I think the text layouting is broken somehow. So I made the text 
shorter, and split it into two, nicely centered in the middle of the plasmoid 
(see screenshot in blog post).

If there is a better way, I'd happily fix it.


- Dominik


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124589/#review83321
-----------------------------------------------------------


On Aug. 2, 2015, 1:59 p.m., Dominik Haumann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124589/
> -----------------------------------------------------------
> 
> (Updated Aug. 2, 2015, 1:59 p.m.)
> 
> 
> Review request for Plasma, Kai Uwe Broulik and Sebastian Kügler.
> 
> 
> Repository: kdeplasma-addons
> 
> 
> Description
> -------
> 
> The disk quota is usually used in enterprise installations where network 
> shares are mounted locally. Typically, sysadmins want to avoid that users 
> copy lots of data into their folders, and therefor set quotas (the quota 
> limit has nothing to do with the physical size of a partition). Typically, 
> once a user gets over the hard limit of the quota, the account is blocked and 
> the user cannot login anymore. This happens from time to time, since the 
> users are not really aware of the current quota limit and the already used 
> disk space.
> 
> Here is where the "Disk Quota" plasmoid helps: It continusouly monitors the 
> disk quota and warns the quota apprpriately.
> 
> A detailed description including screenshots can be found in this blog: 
> http://kate-editor.org/?p=3591
> 
> (I had a KDE4 hack of this plasmoid running at university, and it proved very 
> usable over the years, so it is probably a good idea to have it by default in 
> plasma)
> 
> Issues:
> - the panel icon is larger than the others (some wrong margin?)
> - an icon for the metadata.desktop is missing (the shipped quota.svg file is 
> not available here, it seems).
> - the grid units probably need some more tuning
> 
> 
> Diffs
> -----
> 
>   applets/CMakeLists.txt c60c350 
>   applets/diskquota/CMakeLists.txt PRE-CREATION 
>   applets/diskquota/Messages.sh PRE-CREATION 
>   applets/diskquota/icons/quota.svg PRE-CREATION 
>   applets/diskquota/package/contents/ui/ListDelegateItem.qml PRE-CREATION 
>   applets/diskquota/package/contents/ui/main.qml PRE-CREATION 
>   applets/diskquota/package/metadata.desktop PRE-CREATION 
>   applets/diskquota/plugin/DiskQuota.h PRE-CREATION 
>   applets/diskquota/plugin/DiskQuota.cpp PRE-CREATION 
>   applets/diskquota/plugin/QuotaItem.h PRE-CREATION 
>   applets/diskquota/plugin/QuotaItem.cpp PRE-CREATION 
>   applets/diskquota/plugin/QuotaListModel.h PRE-CREATION 
>   applets/diskquota/plugin/QuotaListModel.cpp PRE-CREATION 
>   applets/diskquota/plugin/plugin.h PRE-CREATION 
>   applets/diskquota/plugin/plugin.cpp PRE-CREATION 
>   applets/diskquota/plugin/qmldir PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124589/diff/
> 
> 
> Testing
> -------
> 
> Tested combinations:
> - no quota installed: A nice message is displayed telling the user that 
> 'quota' is missing.
> - quota installed, but no quota restrictions set: The applet says "No quota 
> restrictions found"
> - quota installed, quotas active: The applet continuously shows the data. The 
> quota entries are in a QAbstractItemModel derived class, so 
> inserting/removing quotas all works (tested).
> - filelight installed: the item under mouse gets highlighted. If clicked, 
> filelight starts with the correct location.
> 
> 
> Thanks,
> 
> Dominik Haumann
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to