Repository navigation
tooling: Link and create checks for guidance process requirements - #625
Conversation
License Check Results🚀 The license check preparation job ran successfully. Status: Click to expand output |
8e764ff to
98b0083
Compare
|
@Aymen-Soussi-01 is this related to your PR?
|
6c360c0 to
584102d
Compare
| source_code_link: "^https://github.com/eclipse-score/score/blob/.*$" | ||
| # Custom semantic validation rules | ||
| custom_validation: | ||
| stop_words: |
There was a problem hiding this comment.
Mhh those are not generic "stop_words". They must not appear in the title. They potentially must appear in the content. So it's really context dependent.
There was a problem hiding this comment.
Fixed. Please resolve.
| - shall | ||
| - must | ||
| - will | ||
| weak_words: |
There was a problem hiding this comment.
What is "weak"? Those are stop words ("prohibited" would be clearer) for the content?!
There was a problem hiding this comment.
Maybe it's something like:
prohibited_words:
id:
- shall
- must
- will
content:
- just
- ...
There was a problem hiding this comment.
Oh and by the way. Especially the filter for content is probably only relevant for requirements, not for all needs?!
There was a problem hiding this comment.
Fixed. Please resolve.
| app.config.needs_extra_links = metamodel["needs_extra_links"] | ||
| app.config.needs_extra_options = metamodel["needs_extra_options"] | ||
| app.config.graph_checks = metamodel["needs_graph_check"] | ||
| app.config.stop_words = metamodel["stop_words"] |
There was a problem hiding this comment.
Q:
Do we need them inside the App config? Will they be used from anywhere else?
There was a problem hiding this comment.
We just need it in the check related but I didnt find an other way to import them without repeating the code of reading the metamodel.yaml file and adding extra code functions
|
@AlexanderLanin Just saw that this PR again fails the 'build' as the checks it introduces are having new findings (since the rebase I guess) and now it again does not go through. I think we have to implement this |
af0b66f to
6605ec5
Compare
|
@Aymen-Soussi-01 please don't amend/squash next time. That makes re-reviews rather impossible. |
|
@hoe-jo this produces ~110 new warnings. Can you have a look and tell whether the guidelines/checks are correct. Either they are wrong, or we will introduce a new non-fatal-warning type for them. |
hoe-jo
left a comment
hoe-jo
left a comment
There was a problem hiding this comment.
I think there was a misunderstanding: Since the weak word check originated from the process requirements of the requirements guidance it should also only be applied to stkh_req, feat_req and comp_req
| optional_options: | ||
| source_code_link: "^https://github.com/eclipse-score/score/blob/.*$" | ||
| # Custom semantic validation rules | ||
| prohibited_words: |
There was a problem hiding this comment.
Please add req-Id to the guidance
There was a problem hiding this comment.
I already added the corresponding req-Id to the related check
=> only requirements, not all elements
=> only requirements, not all elements |
6605ec5 to
3ed722f
Compare
|
Correction: requirement was wrong. It's really only "stkh_req, feat_req and comp_req". @hoe-jo will fix the requirement. |
e7e0fc3 to
8221e32
Compare
|
The created documentation from the pull request is available at: docu-html |
340ac4d to
51c43fe
Compare
Link existing checks via source code linker and implement missing checks. Also-by: Aymen Soussi aymen.soussi@expleogroup.com
51c43fe to
7c0cc6a
Compare
Remove unnecessary added code after merge conflicts with main branch Also-by: Aymen Soussi aymen.soussi@expleogroup.com
| This helps enforce clear and concise naming conventions. | ||
| """ | ||
| stop_words = app.config.stop_words | ||
| if need["type"] in ["stkh_req", "feat_req", "comp_req"]: |
There was a problem hiding this comment.
not sure whether it is a good idea to use hardcoded values.
This should be made configurable so that it is easily expandable.
They could be added as well in the config, something like
# List of requirement types to apply the check to
requirement_types = ["stkh_req", "feat_req", "comp_req"]There was a problem hiding this comment.
this should definitely be in the metamodel.yaml, but I would merge this PR and adjust later.
| --- | ||
| """ | ||
| weak_words = app.config.weak_words | ||
| if need["type"] in [ |
There was a problem hiding this comment.
same here, this is not easily exapnadable
There was a problem hiding this comment.
this should definitely be in the metamodel.yaml, but I would merge this PR and adjust later.
There was a problem hiding this comment.
we will do it later in an other PR as this one took too long and needs to be merged as soon as possible
| log.warning_for_need(need, f"is missing required {field_type}: `{field}`.") | ||
| continue | ||
|
|
||
| if values is None or values in [[], ""]: |
There was a problem hiding this comment.
this block is redundant
do something like
if values in [None, [], ""]:
if required:
log.warning_for_need(need, f"is missing required {field_type}: `{field}`.")
continue # Skip empty optional fields
```
There was a problem hiding this comment.
Fixed. Please resolve
8411433 to
ddd3a62
Compare
ddd3a62 to
4b33d61
Compare
MaximilianSoerenPollak
left a comment
MaximilianSoerenPollak
left a comment
There was a problem hiding this comment.
Further findings and improvements will be implemented in a future PR.
Link existing checks via source code linker and implement missing checks.
Also-by: Aymen Soussi aymen.soussi@expleogroup.com
We decided to disable the optional_links check for now and correct the finding related to that in a separate PR
closes #521