-
Notifications
You must be signed in to change notification settings - Fork 19
drop id and prefix from metamodel.yaml when they are redundant #254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run //src:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR simplifies metamodel configuration by automatically setting default prefix and id values when they are redundant, instead of requiring explicit definitions in the YAML file. The change also standardizes regex patterns to consistently start with ^ to enable future regex identification.
- Removes redundant
prefixandidspecifications from metamodel.yaml where they follow the default pattern - Implements programmatic default setting in the YAML parser with
prefixdefaulting to{directive_name}__andiddefaulting to^{prefix}[0-9a-z_]+$ - Updates regex patterns to consistently start with
^for future pattern identification
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/extensions/score_metamodel/yaml_parser.py | Adds logic to set default prefix and id patterns programmatically |
| src/extensions/score_metamodel/tests/test_metamodel_load.py | Updates test to verify default id pattern is applied |
| src/extensions/score_metamodel/tests/test_check_options.py | Removes unused test method for invalid option value types |
| src/extensions/score_metamodel/metamodel.yaml | Removes redundant prefix and id definitions, adds consistent ^ prefix to regex patterns |
| src/extensions/score_metamodel/checks/check_options.py | Refactors validation logic to return boolean and improve error handling |
| docs/internals/extensions/metamodel.md | Documents default behavior for prefix and id in need types |
Comments suppressed due to low confidence (1)
src/extensions/score_metamodel/metamodel.yaml:1
- This regex pattern is missing the
^anchor at the beginning, making it inconsistent with the stated goal of having all regexes start with^. Should be^assertion__[0-9a-zA-Z_-]*$.
# *******************************************************************************
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
dcalavrezo-qorix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
📌 Description
Once again I got distracted from my actual goal...
The important part here is actually very unnoticeable: The regexes now consistently have a
^as the first character. In future PRs that will be used to identify a regex from a non regex.The more visible change here is that I removed
prefixandidrestrictions where they were obvious. That is, they are now set programmatically during parsing of the yaml. At the very beginning. Therefore the rest of the code is completely unaffected.During review please pay close attention that these match:
prefixandidprefixandidNo new tests added, as this is fully covered by existing tests.
🚨 Impact Analysis
✅ Checklist