User Details
- User Since
- May 6 2015, 11:20 PM (468 w, 2 d)
- Availability
- Available
May 3 2020
One suggestion for this change:
Instead of exporting a method that takes no parameters and always loads from configuration file, why not make a new method with the implementation that takes in a given KConfigGroup. That way unit tests can pass in a KConfigGroup setup appropriately without having to create a normal configuration file in the user's home folder (ie use a temporary file). They can also configure the KConfig to not cascade/use the global configuration so they are isolated from the environment.
Aug 7 2019
LGTM. Regarding the test, if we want to get this change in asap due to the security focus I can submit a follow up patch re-adding it.
Jul 8 2018
Jul 7 2017
+1 LGTM. Before submitting, can you poke the usability people about this change please? If they are also happy, then it's got a ship it from me.
Apr 25 2017
LGTM! Thanks for working on this.
Apr 23 2017
Excellent! Could you add 4 more rows to the tests, to ensure a path without a folder (ex. systemConfigLocation + "/test.desktop") works correctly? Once that's done, it's a ship it from me.
Apr 19 2017
+1 This definitely looks like the correct fix.
Feb 20 2017
Ship it!
I agree, this can't be used so removing it from the API can't hurt anything. Just to be sure, let's take this change for the next release, but not change KConfigBackend' API/ABI until after the next release. If anyone complains about KConfigBackend missing before then, we can just re-export it easily. Otherwise this can always come back in a (ABI) different form if this ever becomes worthwhile. Does that sound good to you?
Feb 4 2017
Dec 1 2016
LGTM!
Sep 10 2016
Feb 19 2016
Out of curiosity, why is the QHash detaching for you here? Or is this just a general fixup found with a linter? Either way, I'm happy to take it :)