Skip to content

kvs: Rename reset_key to set_value_to_default - #585

Open
arkjedrz wants to merge 1 commit into
eclipse-score:mainfrom
qorix-group:arkjedrz_set-value-to-default
Open

kvs: Rename reset_key to set_value_to_default#585
arkjedrz wants to merge 1 commit into
eclipse-score:mainfrom
qorix-group:arkjedrz_set-value-to-default

Conversation

@arkjedrz

@arkjedrz arkjedrz commented Oct 26, 2026

Copy link
Copy Markdown
Contributor
  • Rename in the code, docs, tests.
  • Change the behavior to match the intent of the method.
  • Update docs to match the intent.
  • Drop KeyDefaultNotFound used only by previous version of method.

Fixes #527

@arkjedrz arkjedrz self-assigned this Oct 26, 2026
@github-actions

github-actions Bot commented Oct 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run --lockfile_mode=error //:license-check

Status: ⚠️ Needs Review

Click to expand output 
[License Check Output]
Starting local Bazel server (8.7.0) and connecting to it...
INFO: Invocation ID: e60db9b5-6fa0-42ff-840d-31d2081ae867
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 2 packages loaded
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Loading: 3 packages loaded
Analyzing: target //:license-check (3 packages loaded, 0 targets configured)
Analyzing: target //:license-check (3 packages loaded, 0 targets configured)

Analyzing: target //:license-check (65 packages loaded, 14 targets configured)

Analyzing: target //:license-check (100 packages loaded, 14 targets configured)

Analyzing: target //:license-check (100 packages loaded, 14 targets configured)

Analyzing: target //:license-check (150 packages loaded, 2994 targets configured)

Analyzing: target //:license-check (200 packages loaded, 6523 targets configured)

Analyzing: target //:license-check (205 packages loaded, 11444 targets configured)

Analyzing: target //:license-check (207 packages loaded, 11661 targets configured)

Analyzing: target //:license-check (207 packages loaded, 11661 targets configured)

Analyzing: target //:license-check (208 packages loaded, 11785 targets configured)

Analyzing: target //:license-check (213 packages loaded, 17903 targets configured)

INFO: Analyzed target //:license-check (213 packages loaded, 17903 targets configured).
INFO: From Generating Dash formatted dependency file ...:
INFO: Successfully converted 50 packages from Cargo.lock to bazel-out/k8-fastbuild/bin/tools/formatted.txt
[14 / 16] [Prepa] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar
[15 / 16] Building tools/license.check.license_check.jar (); 0s disk-cache, multiplex-worker
INFO: Found 1 target...
Target //tools:license.check.license_check up-to-date:
  bazel-bin/tools/license.check.license_check
  bazel-bin/tools/license.check.license_check.jar
INFO: Elapsed time: 50.068s, Critical Path: 2.41s
INFO: 16 processes: 12 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 16 total actions
INFO: Running command line: bazel-bin/tools/license.check.license_check tools/formatted.txt <args omitted>
[main] INFO Querying Eclipse Foundation for license data for 42 items.
[main] INFO Found 32 items.
[main] INFO Querying ClearlyDefined for license data for 10 items.
[main] INFO Found 10 items.
[main] INFO License information could not be automatically verified for the following content:
[main] INFO 
[main] INFO crate/cratesio/-/lazy_static/1.5.1
[main] INFO 
[main] INFO This content is either not correctly mapped by the system, or requires review.

@github-actions

github-actions Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Documentation preview for this pull request is available at:
pr-585: https://eclipse-score.github.io/persistency/pr-585/

@arkjedrz
arkjedrz force-pushed the arkjedrz_set-value-to-default branch from 6d2fe8e to e77840a Compare October 26, 2026 09:57
@arkjedrz
arkjedrz deployed to workflow-approval October 26, 2026 09:57 — with GitHub Actions Active
@arkjedrz
arkjedrz deployed to workflow-approval October 26, 2026 09:57 — with GitHub Actions Active
@arkjedrz
arkjedrz force-pushed the arkjedrz_set-value-to-default branch from e77840a to 33f4525 Compare October 27, 2026 09:55
@arkjedrz
arkjedrz deployed to workflow-approval October 27, 2026 09:55 — with GitHub Actions Active
@arkjedrz
arkjedrz deployed to workflow-approval October 27, 2026 09:55 — with GitHub Actions Active
@arkjedrz
arkjedrz force-pushed the arkjedrz_set-value-to-default branch from 33f4525 to 582d1c6 Compare October 28, 2026 06:29
@arkjedrz
arkjedrz force-pushed the arkjedrz_set-value-to-default branch from 582d1c6 to c4a577a Compare October 28, 2026 13:03
@arkjedrz
arkjedrz force-pushed the arkjedrz_set-value-to-default branch from c4a577a to 8646c79 Compare October 28, 2026 15:07
@arkjedrz
kvs: Rename reset_key to set_value_to_default
75a1f91 
- Rename in the code, docs, tests.
  - C++: `ResetKey` to `SetValueToDefault`, drop the deprecated `reset_key`.
- Change the behavior to match the intent of the method.
- Update docs to match the intent.
- Drop `KeyDefaultNotFound` used only by previous version of method.
@arkjedrz
arkjedrz force-pushed the arkjedrz_set-value-to-default branch from 8646c79 to 75a1f91 Compare October 28, 2026 16:10
Ahmed-Elsaka-JC
Ahmed-Elsaka-JC suggested changes Oct 28, 2026
View reviewed changes
Comment thread score/kvs/kvs.cpp

// Return value ignored: a key that was never written needs no reset.
score::cpp::ignore = kvs_.erase(std::string(key));
kvs_.insert_or_assign(std::string(key), default_value);

@Ahmed-Elsaka-JC Ahmed-Elsaka-JC Oct 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would suggent to replace the following two lines :

auto default_value{*default_value_result};
 kvs_.insert_or_assign(std::string(key), default_value);

with :
kvs_.insert_or_assign(std::string(key), std::move(*default_value_result));

To avoid copy value two times.

This branch is waiting to be deployed

1 waiting deployment
workflow-approval — 75a1f91e Waiting Oct 28, 2026 by arkjedrz via qnx-build (score-qnx-aarch64) / approval #1576
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vinodreddy-g vinodreddy-g Awaiting requested review from vinodreddy-g vinodreddy-g is a code owner
@PandaeDo PandaeDo Awaiting requested review from PandaeDo PandaeDo is a code owner
@antonkri antonkri Awaiting requested review from antonkri antonkri is a code owner
@umaucher umaucher Awaiting requested review from umaucher umaucher is a code owner
1 more reviewer
@Ahmed-Elsaka-JC Ahmed-Elsaka-JC Ahmed-Elsaka-JC requested changes
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@arkjedrz arkjedrz

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Resetting key without default value is an error

2 participants

@arkjedrz @Ahmed-Elsaka-JC