Draft #172

Ahmed-Elsaka-JC · 2025-11-21T09:25:05Z

No description provided.

github-actions · 2025-11-21T09:28:56Z

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run //:license-check

Status: ⚠️ Needs Review

Click to expand output

[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.3.0) and connecting to it...
INFO: Invocation ID: 59d72cf7-b9cd-420b-905e-167b83050c14
Computing main repo mapping: 
Computing main repo mapping: 
DEBUG: Rule 'rules_boost+' indicated that a canonical reproducible form can be obtained by modifying arguments integrity = "sha256-LTilyiMMjpYyAi1PW+8RycyAEHy9clo7915R0Bchb2E="
DEBUG: Repository rules_boost+ instantiated at:
  <builtin>: in <toplevel>
Repository rule http_archive defined at:
  /home/runner/.bazel/external/bazel_tools/tools/build_defs/repo/http.bzl:394:31: in <toplevel>
Computing main repo mapping: 
WARNING: For repository 'rules_python', the root module requires module version rules_python@1.4.1, but got rules_python@1.5.1 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'rules_cc', the root module requires module version rules_cc@0.1.1, but got rules_cc@0.2.14 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'googletest', the root module requires module version googletest@1.15.0, but got googletest@1.17.0.bcr.2 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
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: 
Analyzing: target //:license-check (3 packages loaded, 0 targets configured)
Analyzing: target //:license-check (3 packages loaded, 0 targets configured)

Analyzing: target //:license-check (71 packages loaded, 9 targets configured)

Analyzing: target //:license-check (120 packages loaded, 870 targets configured)

Analyzing: target //:license-check (135 packages loaded, 2727 targets configured)

Analyzing: target //:license-check (139 packages loaded, 2857 targets configured)

Analyzing: target //:license-check (139 packages loaded, 2857 targets configured)

Analyzing: target //:license-check (139 packages loaded, 2857 targets configured)

Analyzing: target //:license-check (142 packages loaded, 4864 targets configured)

Analyzing: target //:license-check (142 packages loaded, 4864 targets configured)

Analyzing: target //:license-check (142 packages loaded, 4864 targets configured)

Analyzing: target //:license-check (142 packages loaded, 4864 targets configured)

DEBUG: Rule 'toolchains_llvm++llvm+llvm_toolchain_llvm' indicated that a canonical reproducible form can be obtained by modifying arguments _action_listener = <unknown object com.google.devtools.build.lib.packages.Attribute$LabelListLateBoundDefault>, _config_dependencies = [], _configure = False, _environ = [], _original_name = "llvm_toolchain_llvm"
DEBUG: Repository toolchains_llvm++llvm+llvm_toolchain_llvm instantiated at:
  <builtin>: in <toplevel>
Repository rule llvm defined at:
  /home/runner/.bazel/external/toolchains_llvm+/toolchain/rules.bzl:27:23: in <toplevel>
Analyzing: target //:license-check (142 packages loaded, 4864 targets configured)

INFO: Analyzed target //:license-check (143 packages loaded, 6799 targets configured).
[8 / 14] [Prepa] Expanding template external/score_dash_license_checker+/tool/formatters/dash_format_converter [for tool]
[9 / 14] [Prepa] JavaToolchainCompileClasses external/rules_java+/toolchains/platformclasspath_classes ... (2 actions, 0 running)
INFO: From Generating Dash formatted dependency file ...:
INFO: Successfully converted 65 packages from Cargo.lock to bazel-out/k8-fastbuild/bin/formatted.txt
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 200.475s, Critical Path: 0.64s
INFO: 14 processes: 4 disk cache hit, 9 internal, 1 processwrapper-sandbox.
INFO: Build completed successfully, 14 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

github-actions · 2025-11-21T09:31:33Z

The created documentation from the pull request is available at: docu-html

arkjedrz · 2025-12-18T10:19:30Z

Design/NewDesign.dio

Remove this file.

arkjedrz · 2025-12-18T10:20:22Z

Design/Design_puml.puml

Rename this file (class_diagram_cpp.puml?), move it to docs/, remove Design/ directory.

arkjedrz · 2025-12-18T10:23:06Z

Design/Design_puml.puml

+    BorderColor<<mock>> IndianRed
+}
+
+class IKvsBuilder <<implementation>> {


-> KvsBuilder - no longer an interface.

arkjedrz · 2025-12-18T11:46:21Z

Design/Design_puml.puml

+
+class IKvsBuilder <<implementation>> {
+    - instance_id: InstanceId
+    - need_defaults : bool


-> -defaults : KvsDefaults

Expected values :

true : default storage must be exist

false : default storage may not be exist

Ignores : ignore default values even if we have default values storage.

- new ticket should be created to make c++ support this as rust.

arkjedrz · 2025-12-18T11:46:34Z

Design/Design_puml.puml

+class IKvsBuilder <<implementation>> {
+    - instance_id: InstanceId
+    - need_defaults : bool
+    - need_kvs : bool


-> -kvs_load : KvsLoad

kvs_load = OpenNeedKvs :
OpenNeedKvs has two values now :
new value will be added "IGNORE"

new ticket should be created to make c++ support this as rust.

arkjedrz · 2025-12-18T11:51:43Z

Design/Design_puml.puml

+    + snapshot_count: Result<size_t>
+    + snapshot_max_count() : Result<size_t>
+    + snapshot_restore(snapshot_id:SnapshotId): ResultBlank
+    + get_kvs_file_name(snapshot_id:SnapshotId): Result<filesystem::path>


Remove - filesystem related.

arkjedrz · 2025-12-18T11:53:04Z

Design/Design_puml.puml

+    + set_value(Key:string, value:KvsValue): ResultBlank
+    + remove_key(Key:string) : ResultBlank
+    + flush() : ResultBlank
+    + snapshot_count: Result<size_t>


This is a function.

what is meant by this if a function ?

arkjedrz · 2025-12-18T11:53:21Z

Design/Design_puml.puml

+IStorageBackEnd -[hidden]right- IKvs
+
+' IStorageBackEnd interface (top middle-right)
+interface IStorageBackEnd <<interface>> {


-> IStorageBackend?

arkjedrz · 2025-12-18T11:54:12Z

Design/Design_puml.puml

+' Notes matching original design
+note left of IKvsBuilder
+    builder will contain static map for opened instances
+    single tone should be applied to the builder


What's a single tone?

arkjedrz · 2025-12-18T11:55:52Z

Design/Design_puml.puml

Regarding names - please align with https://github.com/qorix-group/persistency/blob/e3945d4bea1bb3627dd46835db403b0c460306d1/docs/class_diagram.puml

tizava · 2025-12-18T13:13:05Z

Design/Design_puml.puml

Seems like this file is not included in any index file

tizava · 2025-12-18T13:14:01Z

Design/Design_puml.puml

+@startuml NewDesign
+
+' Define color schemes matching original design
+skinparam class {


Why overriding the global design? this will result in an inconsistent documentation.

just to differentiate between Interfaces , implementations, and mocks

tizava · 2025-12-18T13:15:45Z

Design/Design_puml.puml

+    + Load_kvs(instance_id:InstanceId, snapshot_id:SnapshotId):IKvs
+    + Load_default(instance_id:InstanceId)
+    + Flush(instance_id:InstanceId, kvs:unordered_map<string,kvs>
+    + Snapshot_count(instance_id:InstanceId):Result<size_t>


Function name shall follow snake_case convention

as i remember , we should follow PascalCase

tizava · 2025-12-18T13:19:06Z

Design/Design_puml.puml

@@ -0,0 +1,158 @@
+@startuml NewDesign


Remove or rename NewDesign to something more meanigful

tizava · 2025-12-18T13:29:42Z

Design/Design_puml.puml

+    + parser : unique_ptr<IjsonParser> parser
+    + writer : unique_ptr<IjsonWriter> writer
+    + logger : unique_ptr<mw::log::logger> logger
+    + Storage_backend : IStorageBackEnd


Relation (Composition) to IStorageBackEnd is not modelled in this class diagramm

Relation changed from Agregation to composition !!!!

tizava · 2025-12-18T13:30:18Z

Design/Design_puml.puml

+    + default_kvs : unordered_map<string, KvsValue>
+    + filename_prefix : score::filesystem::path
+    + filesystem : score::filesystem
+    + parser : unique_ptr<IjsonParser> parser


Shall the parser and reader not be abstracted by the Backend?

tizava · 2025-12-18T13:34:27Z

Design/Design_puml.puml

+    + snapshot_count: Result<size_t>
+    + snapshot_max_count() : Result<size_t>
+    + snapshot_restore(snapshot_id:SnapshotId): ResultBlank
+    + get_kvs_file_name(snapshot_id:SnapshotId): Result<filesystem::path>


Do we really need them on public interface? If yes, naming is inconsistent file_name vs filename

proposed new design

8a86d85

Ahmed-Elsaka-JC added 2 commits December 18, 2025 00:09

new puml design

e083e16

update dio design

d77abf0

arkjedrz self-requested a review December 18, 2025 07:47

arkjedrz suggested changes Dec 18, 2025

View reviewed changes

tizava suggested changes Dec 18, 2025

View reviewed changes

Draft #172

Are you sure you want to change the base?

Draft #172

Conversation

Ahmed-Elsaka-JC commented Nov 21, 2025

Uh oh!

github-actions bot commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

License Check Results

Uh oh!

github-actions bot commented Nov 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ahmed-Elsaka-JC Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ahmed-Elsaka-JC Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ahmed-Elsaka-JC Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ahmed-Elsaka-JC Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 21, 2025 •

edited

Loading

Ahmed-Elsaka-JC Jan 8, 2026 •

edited

Loading

Ahmed-Elsaka-JC Jan 8, 2026 •

edited

Loading

Ahmed-Elsaka-JC Jan 8, 2026 •

edited

Loading

Ahmed-Elsaka-JC Jan 8, 2026 •

edited

Loading