Review Request 127462: Add support for get QStandardPaths locations.
Matthew Dawson
matthew at mjdsystems.ca
Mon Mar 28 05:39:15 UTC 2016
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127462/#review94057
-----------------------------------------------------------
Much better! I don't think this should handle XDG* variables explicitly, as they won't be used on other platforms and may cause confusion there. On platforms using XDG* variables, Qt handles this for us internally. I opened issues accordingly below. Other then that, looks good to me.
autotests/kconfigtest.cpp (line 581)
<https://git.reviewboard.kde.org/r/127462/#comment64047>
We unfortunately can't test XDG* variables due to Qt, but I think we'll be ok for now. The rest of the looks fine.
docs/options.md (line 94)
<https://git.reviewboard.kde.org/r/127462/#comment64046>
Nitpick: please don't use wildcards here, as it isn't clear to people what variables we are talking about. Instead just point to the list below.
Also, since XDG_*_HOME variables won't be handled on other platforms, we don't need to mention it here.
docs/options.md (line 98)
<https://git.reviewboard.kde.org/r/127462/#comment64049>
I think it be enough to just refer to the enum variable here (ex QStandardsPath::GenericConfigLocation instead of QStandardsPath::writeableLocation(GenericConfigLocation)).
docs/options.md (line 102)
<https://git.reviewboard.kde.org/r/127462/#comment64048>
I think this paragraph can be removed if XDG* isn't handled explicitly.
src/core/kconfig.cpp (line 226)
<https://git.reviewboard.kde.org/r/127462/#comment64045>
This should be handled internally by Qt, so we don't have to explicitly handle this case.
On non-unix platforms, we shouldn't be using those variables as they have their own standard we should integrate against.
- Matthew Dawson
On March 27, 2016, 10:22 a.m., Sandro Knauß wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127462/
> -----------------------------------------------------------
>
> (Updated March 27, 2016, 10:22 a.m.)
>
>
> Review request for KDE Frameworks and Matthew Dawson.
>
>
> Repository: kconfig
>
>
> Description
> -------
>
> Inside desktop files we want to reach also data, cache and config home
> to create files inside these directories.
>
>
> Diffs
> -----
>
> autotests/kconfigtest.h be0a17ea66fbca989a53c68481c4252c9546dd45
> autotests/kconfigtest.cpp e92197f3be57ead47b70ca5d040474e7a554c416
> docs/options.md c7a6c061b700fd7a23b5dd1628cd22a18dec79da
> src/core/kconfig.cpp 07fa6f552c61c52cc1dd64a1c5fb0e2f00873d50
>
> Diff: https://git.reviewboard.kde.org/r/127462/diff/
>
>
> Testing
> -------
>
> Adding tests for XDG_*_HOME variables.
>
>
> Thanks,
>
> Sandro Knauß
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160328/1d111cc0/attachment.html>
More information about the Kde-frameworks-devel
mailing list