process: inspection concept #432

aschemmel-tech · 2025-02-20T14:59:51Z

Ref: closes #321

see also the problem formulation in #567

github-actions · 2025-02-20T15:02:06Z

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

AlexanderLanin · 2025-02-20T20:45:56Z

docs/process/general_concepts/score_review_concept.rst

+It is not the goal to merge this status, but to select the scope of the inspection
+and use the github mechanisms for review comments and fixes of the work product.


Why not make a PR with inspected = true?

Then you will have to change the requirement and the "hash" will be changed again

As Jochen stated, we considered this but ultimately did not follow the idea any more (one reason is above).

I now committed a solution with the status valid and valid(inspected) for requirement and architecture inspection, as this I think is the more natural procedure.

AlexanderLanin · 2025-02-20T20:46:07Z

docs/process/general_concepts/score_review_concept.rst

+In this review the same checklist as for the inspection should be considered, but need not to be filled out.
+
+The last step is initiated by the safety manager, security manager or quality manager:
+He asks the main work product author to set the work product's status to "review" and start a Pull-Request (PR).


The requirement flow is draft -> valid -> review -> valid?

No an additional attibute will be set, but I documented this in the requirement process. Actually I wanted to align this with the operational community

The "review state" will never be merged (this should be prevented by our checks). It is just a means to indicate which requirements shall be inspected (i.e. the scope of the inspection).

removed this from proposal

AlexanderLanin · 2025-02-20T20:46:31Z

docs/process/general_concepts/score_review_concept.rst

+We expect that the work products need to mature during implementation and testing,
+therefore we use a two-step approach for the review and inspection of work products.


So in the first stage we set requirements to valid, although we know they are wrong and will be reworked?

Maybe we need to bring it in context of our branching strategy? Then it will get more clear

It is unrealistic to expect that requirements are written as part of a feature request and expect those will be stable until release of the SW. We do not "know" those are "wrong" as we created those to the best of our knowledge. If we already do a formal inspection at this point, it will most certainly be a waste of time.

we had a look at "best practice" solutions and agreed that for requirements and architecture a two step approach makes sense with a distribution of checks into a first microscopic/incremental review and a later macroscopic/overarching/full inspection. For implementation inspection we would not differentiate.

AlexanderLanin · 2025-02-20T20:47:41Z

docs/process/general_concepts/score_review_concept.rst

+GitHub will automatically ask for a review by the defined one or more "CODEOWNER" of the work product.
+It is not the goal to merge this status, but to select the scope of the inspection
+and use the github mechanisms for review comments and fixes of the work product.
+The author additionally creates an Inspection Document by using the foreseen template.


We've abandoned inspection documents a while ago. Do we really need them in SCORE?

Our initial idea was to comment it into the PR, but this can be altered afterwards ...

Let's have a look at that. I would rather invest into some solution than go for anything manual.

Our safety auditors will not accept an inspection where the result not documented. Any approach which ensures this, I am open to discuss.

Current proposal is to add the checklist to the pull request (e.g. the comments field) in an automated manner.

AlexanderLanin · 2025-02-20T20:48:03Z

docs/process/general_concepts/score_review_concept.rst

+and use the github mechanisms for review comments and fixes of the work product.
+The author additionally creates an Inspection Document by using the foreseen template.
+In this document the inspection result will be documented for each checklist item
+(pass/fail - with link to a ticket for the corrections, or by citing the checklist item in the github comment).


I will try to show an example to help.

AlexanderLanin · 2025-02-20T20:48:58Z

docs/process/general_concepts/score_review_concept.rst

+The author additionally creates an Inspection Document by using the foreseen template.
+In this document the inspection result will be documented for each checklist item
+(pass/fail - with link to a ticket for the corrections, or by citing the checklist item in the github comment).
+The CODEOWNER(s)=reviewers will fill out the checklist document and add their findings on the work product in the PR.


So they have to do the work twice? Sorry, I'm still not sure why normal PR-findings are insufficient.

There is no intention to do double work, but it should be shown also that checklist items were checked as "passed" (which I usually do not expect to see in a PR review).

modified text to make clearer

AlexanderLanin · 2025-02-20T20:51:03Z

docs/process/general_concepts/score_review_concept.rst

+#. Create requirement
+#. (Informal) Pull-Request Review
+#. Merge valid requirement to main
+#. During development and verification steps the requirement is reworked and again put to PR Review


Maybe what you aim at is something like Draft -> Submitted -> Valid?

Or Draft -> Valid -> Reopened -> Valid?

I do not like "draft", as this sounds to me like it is still worked on by the author and not already reviewed.

AlexanderLanin · 2025-02-20T20:53:26Z

docs/process/general_concepts/score_review_concept.rst

+can be based for example for the requiremnts on their "Implemented by", "Verified by" and "Requirement Covered" attribute.
+For example when requesting a new feature by filling out the :need:`GD_TEMP__Feat_Request_Template`
+you are asked to also specify the feature's requirements
+- it is not expected that the maturity of the requirements is already enough at this time to make a good inspection.


This really sounds like those requirements should be "best guess" or "submitted", and not be named "valid"

I looked into our projects. They use: state = enum<draft, assumed, agreed, obsolete>

The flows are e.g. (first state -> next state):

draft -> assumed -> agreed (full flow that as rare as possible)

draft -> agreed (when an old draft is finally agreed by all parties)

assumed -> agreed (when an assumption had to be made, and now finally all parties agree)

agreed (when a new requirement is immediately reviewed by all parties)

This may be ok for requirements where you maybe also have external development, but the process tries to also cover inspections of other work products (architecture, code). But I take the hint and we will talk about the states names in the process community.

hoe-jo · 2025-02-21T13:06:17Z

docs/process/general_concepts/score_review_concept.rst

+also a review checklist is provided for guidance, but no formal inspection is required. The same is true for Safety Analysis and DFA.
+The independence of testing respectively of test case review is covered by the use of GitHub also for the review of test cases.
+Which means that at least the test case definition or the test case review is performed by
+another person as the author of the verified work product.


Do we need to create a process req for it?

But that is already covered by our current configuration, as the review always has to be done by another person.

but now also added several process requirements for the concept - also based on #567

Ref: closes #321

github-actions · 2025-03-11T11:03:16Z

License Check Results

🚀 The license check preparation job ran successfully.

Status: ⚠️ Needs Review

Click to expand output

2025/03/11 11:02:43 Downloading https://releases.bazel.build/7.4.0/release/bazel-7.4.0-linux-x86_64...
Extracting Bazel installation...
Starting local Bazel server and connecting to it...
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: docs
Loading: 0 packages loaded
    currently loading: docs
Loading: 0 packages loaded
    currently loading: docs
Analyzing: target //docs:license.check.python (1 packages loaded)
Analyzing: target //docs:license.check.python (1 packages loaded, 0 targets configured)
Analyzing: target //docs:license.check.python (1 packages loaded, 0 targets configured)

Analyzing: target //docs:license.check.python (90 packages loaded, 10 targets configured)

Analyzing: target //docs:license.check.python (93 packages loaded, 10 targets configured)

Analyzing: target //docs:license.check.python (97 packages loaded, 10 targets configured)

Analyzing: target //docs:license.check.python (130 packages loaded, 818 targets configured)

Analyzing: target //docs:license.check.python (131 packages loaded, 826 targets configured)

Analyzing: target //docs:license.check.python (144 packages loaded, 2461 targets configured)

Analyzing: target //docs:license.check.python (144 packages loaded, 2465 targets configured)

Analyzing: target //docs:license.check.python (145 packages loaded, 2589 targets configured)

Analyzing: target //docs:license.check.python (148 packages loaded, 4611 targets configured)

INFO: Analyzed target //docs:license.check.python (149 packages loaded, 4736 targets configured).
[11 / 13] [Prepa] JavaToolchainCompileBootClasspath external/rules_java~/toolchains/platformclasspath.jar
[12 / 13] Building docs/license.check.python.jar (); 0s multiplex-worker
INFO: Found 1 target...
Target //docs:license.check.python up-to-date:
  bazel-bin/docs/license.check.python
  bazel-bin/docs/license.check.python.jar
INFO: Elapsed time: 23.171s, Critical Path: 2.75s
INFO: 13 processes: 9 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 13 total actions
INFO: Running command line: bazel-bin/docs/license.check.python docs/formatted.txt -review -project automotive.score -repo https://github.com/eclipse-score/score -token otyhZ4eaRYK1tKLNNF-Y
[main] INFO Querying Eclipse Foundation for license data for 69 items.
[main] INFO Found 45 items.
[main] INFO Querying ClearlyDefined for license data for 42 items.
[main] ERROR Error response from ClearlyDefined 429

hoe-jo

ok for an initial proposal

aschemmel-tech requested a review from hoe-jo February 20, 2025 15:00

aschemmel-tech force-pushed the aschemmel-tech-patch-3 branch from 654ee4d to 00753ed Compare February 20, 2025 15:05

AlexanderLanin reviewed Feb 20, 2025

View reviewed changes

hoe-jo reviewed Feb 21, 2025

View reviewed changes

aschemmel-tech added 2 commits March 11, 2025 08:56

process: inspection concept

9cc6bf5

Ref: closes #321

process: inspection concept - version 2

c9ff682

Ref: closes #321

aschemmel-tech force-pushed the aschemmel-tech-patch-3 branch from 00753ed to c9ff682 Compare March 11, 2025 11:02

aschemmel-tech marked this pull request as ready for review March 11, 2025 13:15

aschemmel-tech mentioned this pull request Mar 11, 2025

Fix audit 2 findings #537

Closed

52 tasks

hoe-jo self-requested a review March 12, 2025 13:38

hoe-jo approved these changes Mar 12, 2025

View reviewed changes

aschemmel-tech merged commit 9d774e7 into main Mar 12, 2025
10 checks passed

aschemmel-tech deleted the aschemmel-tech-patch-3 branch March 12, 2025 13:39

		It is not the goal to merge this status, but to select the scope of the inspection
		and use the github mechanisms for review comments and fixes of the work product.

		We expect that the work products need to mature during implementation and testing,
		therefore we use a two-step approach for the review and inspection of work products.

process: inspection concept #432

process: inspection concept #432

Uh oh!

Conversation

aschemmel-tech commented Feb 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 20, 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!

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!

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!

AlexanderLanin Feb 21, 2025 • 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!

aschemmel-tech Mar 11, 2025 • 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!

github-actions bot commented Mar 11, 2025

License Check Results

Uh oh!

hoe-jo left a comment

Choose a reason for hiding this comment

aschemmel-tech commented Feb 20, 2025 •

edited

Loading

AlexanderLanin Feb 21, 2025 •

edited

Loading

aschemmel-tech Mar 11, 2025 •

edited

Loading