Update new-backend docs #4573

kodebach · 2022-10-18T16:39:29Z

Incorporates the reviews from #4187

Basics

Short descriptions of your changes are in the release notes
(added as entry in doc/news/_preparation_next_release.md which
contains _(my name)_)
Please always add something to the release notes.
Details of what you changed are in commit messages
(first line should have module: short statement syntax)
References to issues, e.g. close #X, are in the commit messages.
The buildservers are happy. If not, fix in this order:
- add a line in doc/news/_preparation_next_release.md
- reformat the code with scripts/dev/reformat-all
- make all unit tests pass
- fix all memleaks
The PR is rebased with current master.

Checklist

I added unit tests for my code
I fully described what my PR does in the documentation
(not in the PR description)
I fixed all affected documentation (see Documentation Guidelines)
I added code comments, logging, and assertions as appropriate (see Coding Guidelines)
I updated all meta data (e.g. README.md of plugins and METADATA.ini)
I mentioned every code not directly written by me in reuse syntax

Review

Documentation is introductory, concise, good to read and describes everything what the PR does
Examples are well chosen and understandable
Code is conforming to our Coding Guidelines
APIs are conforming to our Design Guidelines
Code is consistent to our Design Decisions

Labels

Add the "work in progress" label if you do not want the PR to be reviewed yet.
Add the "ready to merge" label if the basics are fulfilled and no further pushes are planned by you.

src/libs/elektra/kdb.c

markus2330

Great job! Keep up the good work! ❤️

doc/decisions/backend_plugin.md

doc/decisions/commit_function.md

doc/dev/backend-plugins.md

src/libs/elektra/kdb.c

markus2330 · 2022-10-22T05:36:28Z

src/libs/elektra/kdb.c

- * **Note**: In general it is recommended to use a @p parentKey in the cascading namespace.
+ * - `spec:/`, `dir:/`, `user:/` and `system:/` can be loaded via kdbGet().
+ * - `proc:/` keys can be loaded via kdbGet(), but are not persisted or cached.
+ * - `default:/` keys can be returned by kdbGet() but they will always stem from a specification in `spec:/` keys


To not make a constraint to default: keys added beforehand:

Suggested change

* - `default:/` keys can be returned by kdbGet() but they will always stem from a specification in `spec:/` keys

* - `default:/` keys can be inserted by kdbGet() but they will always stem from a specification in `spec:/` keys

To not make a constraint to default: keys added beforehand

I accepted the suggestion, but not sure what you wanted to say here. I thought we want default:/ keys to come from spec:/ only (see #4484 (comment)). Pre-existing default:/ keys will be removed, if the overlap with a loaded backend. Outside those backends, they will of course be kept, but that's the case for any other namespace too.

The change is good then. But it would be good to have these two sentences in the docu, too.

src/libs/elektra/kdb.c

markus2330 · 2022-10-22T05:39:13Z

src/plugins/backend/README.md

-/positions/set/commit = (="#0")
-/positions/set/rollback (="#0")
-```
+<!-- TODO [new_backend]: finish README -->


👍 please

src/plugins/missing/README.md

src/libs/elektra/kdb.c

kodebach · 2022-10-24T11:56:02Z

@markus2330 Didn't have time yet to create a README for backend, but hopefully I addressed all the rest.

Regarding the decisions here:

commit_function.md: This one is the important one that should be decided ASAP. I'm heavily in favour of not having separate functions (i.e. option 3 or 4). I would prefer the separate phase arguments, but that probably breaks to much stuff, so I'd go with option 3.
plugin_contract_function.md: First of all, I know option 3 here, is not really part of this decision. But it didn't quite fit here and I just wanted to quickly write down the idea. I will remove it before the decision goes any further. This issue should also be decided soon. I prefer the void ELEKTRA_PLUGIN_EXPORT (KeySet * ks) variant, because of all the other things it makes possible.
plugin_struct.md: That's basically a list of ideas I had while looking into the other two decisions. In short, I think there is a lot of potential to slim down the struct _Plugin and improve some semi-related things. Please, ignore this file for now. I just wanted to write down the ideas.

Note I won't do any more work on these decisions in this PR. Feel free to comment, but I will create separate PRs for each of them once this PR is merged.

markus2330 · 2022-10-26T17:59:14Z

Didn't have time yet to create a README for backend,

Would be good to have it!

but hopefully I addressed all the rest.

What about the other reviews? Are they also incorporated here?

I will create separate PRs for each of them once this PR is merged.

Excellent idea!

kodebach · 2022-10-26T18:04:03Z

What about the other reviews? Are they also incorporated here?

Yes, everything from #4187 should be addressed.

markus2330

Still a few little things open. @lawli3t can you take a look at the API docu, too?

src/libs/elektra/kdb.c

markus2330

LGTM, many improvements. Decisions will be handled later anyway. Please rebase.

doc/decisions/plugin_struct.md

Co-authored-by: Maximilian Irlinger <maxi6594@gmail.com>

Co-authored-by: Markus Raab <markus2330@users.noreply.github.com>

kodebach · 2022-11-10T12:55:29Z

@markus2330 rebased, please merge when CI completes

markus2330 · 2022-11-10T17:30:44Z

The build did not succeed.

markus2330 · 2022-11-10T18:36:06Z

Please don't change the code in this already-reviewed documentation PR.

kodebach · 2022-11-10T18:55:52Z

Please don't change the code in this already-reviewed documentation PR.

It's my PR, please don't tell me what to do with it...

I intentionally, split 85e8e19 and 3694bb8.

The first one was a clear bug and I don't think there can be any objection to the fix. It even matches, what the docs already say. The second one does add new functionality, whether or not we want to add it is up for debate. But that's why I requested a new review. Well now I did... You commented faster than I could request the review...

markus2330

Please split documentation and code in this PR. In the current form I cannot review it anymore.

kodebach · 2022-11-11T11:03:33Z

I have now also remove the minor fix for "kdbGet never returns 0", because for some unexplained reason the test didn't work for the FULL build. I believe you can now just merge this PR, when the CI is done. There are now changes since your last review.

EDIT: I also created #4673 to track the issue

markus2330 · 2022-11-12T07:48:27Z

jenkins build libelektra please

markus2330 · 2022-11-12T09:12:51Z

Great work, thank you! 💞

kodebach requested a review from markus2330 October 18, 2022 16:39

atmaxinger reviewed Oct 18, 2022

View reviewed changes

src/libs/elektra/kdb.c Outdated Show resolved Hide resolved

markus2330 suggested changes Oct 22, 2022

View reviewed changes

kodebach commented Oct 24, 2022

View reviewed changes

src/libs/elektra/kdb.c Outdated Show resolved Hide resolved

kodebach commented Oct 24, 2022

View reviewed changes

src/libs/elektra/kdb.c Outdated Show resolved Hide resolved

markus2330 suggested changes Oct 26, 2022

View reviewed changes

markus2330 reviewed Oct 27, 2022

View reviewed changes

src/libs/elektra/kdb.c Outdated Show resolved Hide resolved

markus2330 mentioned this pull request Nov 1, 2022

logchange: add notification-send hooks to contract #4622

Merged

22 tasks

kodebach requested a review from markus2330 November 1, 2022 14:28

markus2330 approved these changes Nov 3, 2022

View reviewed changes

doc/decisions/plugin_struct.md Outdated Show resolved Hide resolved

kodebach and others added 12 commits November 10, 2022 13:49

incorporate reviews from ElektraInitiative#4187

d304997

Update src/libs/elektra/kdb.c

0428bd8

Co-authored-by: Maximilian Irlinger <maxi6594@gmail.com>

update man pages

83422c4

Apply suggestions from code review

f741e70

Co-authored-by: Markus Raab <markus2330@users.noreply.github.com>

Update src/libs/elektra/kdb.c

12c6955

Update doc/decisions/backend_plugin.md

399302d

Co-authored-by: Markus Raab <markus2330@users.noreply.github.com>

update decisions

a1c488c

update release notes

293c6cc

fix formatting

faefd53

Apply suggestions from code review

b2bdbf7

Co-authored-by: Markus Raab <markus2330@users.noreply.github.com>

update docs

0d163dc

move new decisions to drafts

21247d4

kodebach force-pushed the new-backend-docs branch from 26b68ca to 21247d4 Compare November 10, 2022 12:51

kodebach added 2 commits November 10, 2022 13:52

decisions: fix half sentence

a526a85

fix formatting

c8ee11d

doc: fix links

ad1db9d

kodebach requested a review from markus2330 November 10, 2022 18:54

markus2330 suggested changes Nov 11, 2022

View reviewed changes

kodebach force-pushed the new-backend-docs branch from 3694bb8 to 85e8e19 Compare November 11, 2022 10:04

This comment was marked as outdated.

Sign in to view

kodebach force-pushed the new-backend-docs branch from 85e8e19 to ad1db9d Compare November 11, 2022 11:01

markus2330 merged commit eb0704c into ElektraInitiative:master Nov 12, 2022

mpranj added this to the 0.9.12 milestone Jan 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update new-backend docs #4573

Update new-backend docs #4573

kodebach commented Oct 18, 2022

markus2330 left a comment

markus2330 Oct 22, 2022

kodebach Oct 24, 2022

markus2330 Oct 26, 2022

markus2330 Oct 22, 2022

kodebach commented Oct 24, 2022

markus2330 commented Oct 26, 2022

kodebach commented Oct 26, 2022

markus2330 left a comment

markus2330 left a comment

kodebach commented Nov 10, 2022

markus2330 commented Nov 10, 2022

markus2330 commented Nov 10, 2022

kodebach commented Nov 10, 2022

markus2330 left a comment

This comment was marked as outdated.

kodebach commented Nov 11, 2022 •

edited

Loading

markus2330 commented Nov 12, 2022

markus2330 commented Nov 12, 2022

	* - `default:/` keys can be returned by kdbGet() but they will always stem from a specification in `spec:/` keys
	* - `default:/` keys can be inserted by kdbGet() but they will always stem from a specification in `spec:/` keys

Update new-backend docs #4573

Update new-backend docs #4573

Conversation

kodebach commented Oct 18, 2022

Basics

Checklist

Review

Labels

markus2330 left a comment

Choose a reason for hiding this comment

markus2330 Oct 22, 2022

Choose a reason for hiding this comment

kodebach Oct 24, 2022

Choose a reason for hiding this comment

markus2330 Oct 26, 2022

Choose a reason for hiding this comment

markus2330 Oct 22, 2022

Choose a reason for hiding this comment

kodebach commented Oct 24, 2022

markus2330 commented Oct 26, 2022

kodebach commented Oct 26, 2022

markus2330 left a comment

Choose a reason for hiding this comment

markus2330 left a comment

Choose a reason for hiding this comment

kodebach commented Nov 10, 2022

markus2330 commented Nov 10, 2022

markus2330 commented Nov 10, 2022

kodebach commented Nov 10, 2022

markus2330 left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

kodebach commented Nov 11, 2022 • edited Loading

markus2330 commented Nov 12, 2022

markus2330 commented Nov 12, 2022

kodebach commented Nov 11, 2022 •

edited

Loading