Conversation
added warning when elektra is enabled and ini backend is used added flag to skip tests which fail on the test machine
moved kdb and keyset to backend instances
# Conflicts: # src/core/kcoreconfigskeleton.h
|
Can you please properly pull to master (from upstream master), rebase your branch and create a new PR? The PR should only contain your changes. |
|
Okay, i'm currently pushing a new branch and will open the pr shortly. I had to pull in the upstream master, because they added some changes on which other applications depended. But this should work, once we merge with our master. |
Yes, you can and should pull upstream master. But please push it directly to our master, not within your PR. |
| * @param parent the iterator of the parent key | ||
| * @param child the iterator of the child key | ||
| */ | ||
| inline void traverseIterators(Key::iterator* parent, Key::iterator* child) { |
There was a problem hiding this comment.
Parameters can be passed as reference instead of pointer
| std::string key; | ||
| }; | ||
|
|
||
| inline KConfigKey elektraKeyToKConfigKey(Key::iterator* key, Key::iterator* end) { |
There was a problem hiding this comment.
If you're not intending to manipulate an existing iterator, it would be safer to pass as value. Key::iterator (or NameIterator) only contains 3 pointers, so it won't introduce any noticeable overhead
| Key key; | ||
|
|
||
|
|
||
| while((key = keySet.pop()) != nullptr) { |
There was a problem hiding this comment.
can for(auto& key : keySet) be used here?
There was a problem hiding this comment.
I had to change it to const but it works (for(const auto& key : keySet))
| return ParseOk; | ||
| } | ||
|
|
||
| inline std::string kConfigGroupToElektraKey(std::string group, const std::string& keyname) { |
There was a problem hiding this comment.
This might introduce some trouble. In kconfig config files key names may contain the / character. I used ckdb::Key.setBaseName to append key/group names to a ckdb::Key so that it escapes characters automatically.
There was a problem hiding this comment.
okay, changed it
I pushed the changes to the branch of #14
There was a problem hiding this comment.
maybe lets merge #14 and close this PR. It is a bit confusing now.
|
@FelixResch please try not to keep important PRs open too long as this hinders others to successfully work with the master of this repo. What is the status here? Can you please rebase? |
3 participants
Master from upstream is already merged. This was necessary, as there were breaking changes in KF5 which were already used by other applications.
Code has been formatted according to KDE specification (https://community.kde.org/Policies/Frameworks_Coding_Style)
There currently are some dirty hacks in it, but the basic system is now working.