Conversation
9088c76 to
a455217
Compare
masc2023
left a comment
There was a problem hiding this comment.
Why not follow process description, that will cover also other standards, Safety is based on Quality, and defined folder structures?
0f3a3f4 to
74365d5
Compare
8421a28 to
ae85726
Compare
The concept behind the module API is to enforce that modules follow the score processes. |
1744747 to
04fe3f6
Compare
AlexanderLanin
left a comment
There was a problem hiding this comment.
Very good start! I feel using baselibs as an example is not ideal. We should try to apply this to a component that has existing documentation.
| safety_element_out_of_context( | ||
| name = "my_module", | ||
| assumptions_of_use = ":assumptions", | ||
| component_requirements = ":requirements", |
There was a problem hiding this comment.
That would be seooc_requirements... but since seeoc is redundant we can also just say requirements? But that again is confusing. Is it maybe assumed requirements? Or what is it? Probably a question for the process experts :)
There was a problem hiding this comment.
IMHO, we have feature requirements and component requirements. Conceptually, its important that these requirements are verified with the targets from tests. So that we can compute a meaningful requirements coverage.
There was a problem hiding this comment.
That would be
seooc_requirements... but since seeoc is redundant we can also just sayrequirements? But that again is confusing. Is it maybe assumed requirements? Or what is it? Probably a question for the process experts :)
Process defines assumptions of use and requirementes, e.g.Platform
https://eclipse-score.github.io/score/main/requirements/index.html
|
|
||
| - ``name``: The name of the safety element module. Used as the base name | ||
| for all generated targets. | ||
| - ``assumptions_of_use``: Label to a ``.rst`` or ``.md`` file containing the |
There was a problem hiding this comment.
I imagine that this may not be a single file. And what exactly is a file here? filegroup?
There was a problem hiding this comment.
Valid point. Technically it can be anything which returns a SphinxDocsLibraryProvider, which of course can contain rst, md, svg files etc.
| safety analysis, including FMEA, FMEDA, FTA, or other safety analysis | ||
| results as required by ISO 26262-9 clause 8. Documents hazard analysis and | ||
| safety measures. | ||
| - ``implementations``: List of labels to Bazel targets representing the actual |
There was a problem hiding this comment.
bazel targets would be treated as transitive?! So why do we even need multiple labels here?
There was a problem hiding this comment.
You are right. We could hide this behind a single target. But this is what the rule already does. The idea was a different one: Behind each of these targets is an api, provided to the user. E.g. the hdrs attribute of a cc_library provide the header files published by that library. We should also use this information to enhance the documentation with API docs generated from the code.
| software implementation (cc_library, cc_binary, etc.) that realizes the | ||
| component requirements. This is the source code that implements the safety | ||
| functions as required by ISO 26262-6 clause 8. | ||
| - ``tests``: List of labels to Bazel test targets (cc_test, py_test, etc.) |
There was a problem hiding this comment.
Remark for the future: we'll need to split this to slow tests and fast tests, or the old unit test and integration test split.
There was a problem hiding this comment.
I agree. But then we should differentiate between "integration tests" and "unit tests" according to the test lavels.
|
|
||
| **Generated Targets:** | ||
|
|
||
| This macro creates multiple targets automatically: |
There was a problem hiding this comment.
These are all docu related. Where are implementation and tests?
| - ``assumptions_of_use``: Label to a ``.rst`` or ``.md`` file containing the | ||
| Assumptions of Use, which define the safety-relevant operating conditions | ||
| and constraints for the SEooC as required by ISO 26262-10 clause 5.4.4. | ||
| - ``component_requirements``: Label to a ``.rst`` or ``.md`` file containing |
There was a problem hiding this comment.
SEOOC would contain only safety requirements AFAIK. Do we want to include all requirements here?
There was a problem hiding this comment.
Yes. A limitation to safety requirements does not make sense. The safety_element_out_of_context shall be a self contained unit.
There was a problem hiding this comment.
Just to avoid here misunderstanding, Process covers NOT only Safety, also QM and Security, and we define here Modules, containing everything, Module can be seen as SEooC, but compare Handbook and Safety Plan, we do not deliver SEooC, there will be no safey argumentation, integrator can take it to make a SEooC out it.
https://eclipse-score.github.io/process_description/main/general_concepts/score_building_blocks_concept.html
https://eclipse-score.github.io/process_description/main/general_concepts/score_building_blocks_concept.html
https://eclipse-score.github.io/process_description/main/process_areas/safety_management/safety_management_workproducts.html#wp__platform_safety_package
There was a problem hiding this comment.
So the basic naming is wrong here? It should not be "safety_element_out_of_context" but just "score_module"?
| "assumptions_of_use": attr.label( | ||
| providers = [SphinxDocsLibraryInfo], | ||
| mandatory = True, | ||
| doc = "Label to a sphinx_docs_library target containing the Assumptions of Use, which define the safety-relevant operating conditions and constraints for the SEooC as required by ISO 26262-10 clause 5.4.4.", |
There was a problem hiding this comment.
Ah, this doesn't quite match docu where it says this needs to point to an rst/md file.
There was a problem hiding this comment.
Fair. the documentation needs to be updated.
| all_files = [] | ||
| for artifact in seooc_artifacts: | ||
| dep = getattr(ctx.attr, artifact) | ||
| for t in dep[SphinxDocsLibraryInfo].transitive.to_list(): |
There was a problem hiding this comment.
This assembles all the artifacts into a standard file structure. So I have wo wonder, whether it would be easier to collect only a single SphinxDocsLibraryInfo, instead of collecting multiple and internally assembling a single one.
There was a problem hiding this comment.
yes. This is intenionally like this. The rule shall provide an abstract api, which always provides you a valid SpinxDocsLibrary.
| mandatory = True, | ||
| doc = "Label to a sphinx_docs_library target containing the safety analysis, including FMEA, FMEDA, FTA, or other safety analysis results as required by ISO 26262-9 clause 8. Documents hazard analysis and safety measures.", | ||
| ), | ||
| } |
There was a problem hiding this comment.
Having everything in separate SphinxDocsLibraryInfo targets means the generated index file will differ from the original index file. So module docu will look different than seeoc docu of the same module. Correct?
bazel/rules/score_module/test/BUILD
Outdated
| package(default_visibility = ["//visibility:public"]) | ||
|
|
||
| # Test fixtures for seooc tests | ||
| sphinx_docs_library( |
There was a problem hiding this comment.
This would currently only provide the raw .rst files? Specifically no prebuild json data?
ramceb
left a comment
There was a problem hiding this comment.
Very good start! I feel using baselibs as an example is not ideal. We should try to apply this to a component that has existing documentation.
Thx. Baselibs was chosen on purpose as it provides several seoocs. Anyhow, we can also use any other module, which comes with more documentation in the module repository. Any suggestions?
Persistency? |
3485763 to
267ddf8
Compare
|
@AlexanderLanin and @masc2023 Sorry for the delay. I (force) pushed a second commit and took most of you comments into account. Main change is that we are now able to do modular sphinx builds. So when you build a score_module bazel will cache the generated html files of dependent modules and merge the files later into the html tree of the root module. This way link resolution works and you get always a complete set of html files with all submodules. So there is now a generic score_module and a more specific safety_element_out_of_context, which enforces a common format for documenation. We can discuss if we want to enforce this or not. |
a02bb9d to
ebb600e
Compare
bazel/rules/rules_score/test/fixtures/seooc_test/architectural_design.rst
Show resolved
Hide resolved
bazel/rules/rules_score/test/fixtures/seooc_test/dependability_analysis.rst
Show resolved
Hide resolved
bazel/rules/rules_score/test/fixtures/seooc_test/dependability_analysis.rst
Show resolved
Hide resolved
ramceb
left a comment
There was a problem hiding this comment.
Example based on persistency is here: eclipse-score/persistency#235
| # SPDX-License-Identifier: Apache-2.0 | ||
| # ******************************************************************************* | ||
|
|
||
| """ |
There was a problem hiding this comment.
I suggest to change into "Safety analyses" as both DFA & FMEAs fall into this naming in ISO 26262. FMEA is not (only) a dependability analysis.
There was a problem hiding this comment.
Propose to keep it, later with add also security analyses, so dependable is the abstraction of both
FScholPer
left a comment
There was a problem hiding this comment.
blocked by eclipse-score/score#2346
This commit introduces the initial implementation of the score_module
Bazel rules for handling SEOOC (Safety Element Out Of Context) modules.
Key additions:
- Core rules in score_module.bzl and private/seooc.bzl
- Test infrastructure with fixtures covering architecture design,
assumptions of use, component requirements, and safety analysis
Incorporated first feedback. Introduced a modular sphinx-build.
Parameter renaming
@FScholPer Is this really a blocker. For the implementation it doesn't matter if in which repos the features live in. |
|
@FScholPer The PR was discussed in the process community and accepted for merge: https://github.com/orgs/eclipse-score/discussions/407#discussioncomment-15605348 Also the PR you mentioned was discussed but no blocker was identified here! |
masc2023
left a comment
There was a problem hiding this comment.
As discussed initial version, fine for now, updates and fine tuning to be done in further PRs
| # SPDX-License-Identifier: Apache-2.0 | ||
| # ******************************************************************************* | ||
|
|
||
| """ |
There was a problem hiding this comment.
Propose to keep it, later with add also security analyses, so dependable is the abstraction of both
This commit introduces the initial implementation of the score_module
Bazel rules for handling SEOOC (Safety Element Out Of Context) modules.
Key additions:
assumptions of use, component requirements, and safety analysis