-
Notifications
You must be signed in to change notification settings - Fork 1
feat(curations): Start split of classes in proper locations #11
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
Signed-off-by: Helio Chissini de Castro <helio.chissini.de.castro@cariad.technology>
Initial repository configuration implementation allocated all enum and model classes in a single file location. Even being functional, it was diverging from Ort main Kotlin representation, so a proper split was necessary. Started with the curations model objects, and is the template for the next iterations. Signed-off-by: Helio Chissini de Castro <helio.chissini.de.castro@cariad.technology>
Signed-off-by: Helio Chissini de Castro <helio.chissini.de.castro@cariad.technology>
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 bumps the version to 0.3.0 and refactors the codebase to remove the pyyaml dependency. The changes introduce new structured Pydantic models for VCS information, hash algorithms, and package/license curations, replacing previously loosely-typed configurations.
Key Changes:
- Removed
pyyamldependency from project dependencies - Added new Pydantic model classes for VCS, Hash, and Curation configurations
- Refactored repository configuration to use new structured models
- Updated ruff from 0.14.3 to 0.14.4
- Consolidated schema files (deleted separate curations-schema.json)
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Bumped version to 0.3.0, removed pyyaml dependency, updated ruff to 0.14.4, added Python 3.14 classifier |
| uv.lock | Updated lock file to reflect dependency changes |
| src/ort/models/vcstype.py | New model for VCS type with aliases support |
| src/ort/models/vcsinfo.py | New model for VCS information (type, URL, revision, path) |
| src/ort/models/hash_algorithm.py | New enum for hash algorithms |
| src/ort/models/hash.py | New model bundling hash algorithm and value |
| src/ort/models/package_curation_data.py | New model for package curation data |
| src/ort/models/package_curation.py | New model for package curation |
| src/ort/models/config/license_finding_curation_reason.py | New enum for license finding curation reasons |
| src/ort/models/config/license_finding_curation.py | New model for license finding curations |
| src/ort/models/config/curations.py | New unified curations model |
| src/ort/models/repository_configuration.py | Refactored to use new curation models, removed duplicate class definitions |
| schemas/repository-configuration-schema.json | Updated with new model definitions |
| schemas/curations-schema.json | Deleted (consolidated into repository-configuration-schema.json) |
| .pre-commit-config.yaml | Updated ruff version to 0.14.4 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| SHA1GIT = ( | ||
| ["SHA-1-GIT", "SHA1-GIT", "SHA1GIT", "SWHID"], | ||
| "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391", | ||
| ) |
Copilot
AI
Nov 7, 2025
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.
The SHA1GIT enum value is defined as a tuple instead of a string like other enum values. This is inconsistent with the enum pattern and will cause the enum value to be (['SHA-1-GIT', 'SHA1-GIT', 'SHA1GIT', 'SWHID'], 'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391') instead of a string. If aliases are needed, consider using a different pattern or just use a single representative string value like the other entries.
| SHA1GIT = ( | |
| ["SHA-1-GIT", "SHA1-GIT", "SHA1GIT", "SWHID"], | |
| "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391", | |
| ) | |
| SHA1GIT = "SHA1GIT" |
| ) | ||
| path: str = Field( | ||
| default="", | ||
| description="The path inside the VCS to take into account." |
Copilot
AI
Nov 7, 2025
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.
Missing space after the period in the description. Should be 'The path inside the VCS to take into account. If the VCS supports...' (note the space after the period).
| description="The path inside the VCS to take into account." | |
| description="The path inside the VCS to take into account. " |
| ) | ||
| start_lines: list[int] | None = Field( | ||
| default=None, | ||
| description="A matcher for the start line of a license finding, matches if the start line matches any of" |
Copilot
AI
Nov 7, 2025
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.
Missing space after 'of' in the description. Should be 'matches any of [startLines]' (note the space before the bracket).
| description="A matcher for the start line of a license finding, matches if the start line matches any of" | |
| description="A matcher for the start line of a license finding, matches if the start line matches any of " |
| description="A matcher for the line count of a license finding" | ||
| "matches if the line count equals [lineCount] or if [lineCount] is None", |
Copilot
AI
Nov 7, 2025
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.
Missing space after 'finding' in the description. Should be 'A matcher for the line count of a license finding matches...' with a space or better yet, add proper sentence separation.
| description="A matcher for the line count of a license finding" | |
| "matches if the line count equals [lineCount] or if [lineCount] is None", | |
| description="A matcher for the line count of a license finding. " | |
| "Matches if the line count equals [lineCount] or if [lineCount] is None", |
|
|
||
| detected_license: str | None = Field( | ||
| default=None, | ||
| description="The concluded license as SPDX expression or None" |
Copilot
AI
Nov 7, 2025
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.
Missing space after 'None' in the description. Should be 'or None for no license' (note the space before 'for').
| description="The concluded license as SPDX expression or None" | |
| description="The concluded license as SPDX expression or None " |
| "pytest>=8.4.2", | ||
| "ruff>=0.14.3", | ||
| "ruff>=0.14.4", | ||
| "types-pyyaml>=6.0.12.20250915", |
Copilot
AI
Nov 7, 2025
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.
The 'types-pyyaml' package is still listed in dev dependencies, but 'pyyaml' has been removed from the main dependencies. Since the codebase no longer uses pyyaml, the type stubs are also unnecessary and should be removed.
| "types-pyyaml>=6.0.12.20250915", |
| } | ||
| ], | ||
| "default": null, | ||
| "description": "A matcher for the start line of a license finding, matches if the start line matches any of[startLines] or if [startLines] is empty.", |
Copilot
AI
Nov 7, 2025
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.
Missing space after 'of' in the description. Should be 'matches any of [startLines]' (note the space before the bracket).
| "description": "A matcher for the start line of a license finding, matches if the start line matches any of[startLines] or if [startLines] is empty.", | |
| "description": "A matcher for the start line of a license finding, matches if the start line matches any of [startLines] or if [startLines] is empty.", |
| } | ||
| ], | ||
| "default": null, | ||
| "description": "A matcher for the line count of a license findingmatches if the line count equals [lineCount] or if [lineCount] is None", |
Copilot
AI
Nov 7, 2025
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.
Missing space between 'finding' and 'matches'. Should be 'A matcher for the line count of a license finding matches...'.
| "description": "A matcher for the line count of a license findingmatches if the line count equals [lineCount] or if [lineCount] is None", | |
| "description": "A matcher for the line count of a license finding matches if the line count equals [lineCount] or if [lineCount] is None", |
| } | ||
| ], | ||
| "default": null, | ||
| "description": "The concluded license as SPDX expression or Nonefor no license, see https://spdx.dev/spdx-specification-21-web-version#h.jxpfx0ykyb60.", |
Copilot
AI
Nov 7, 2025
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.
Missing space between 'None' and 'for'. Should be 'or None for no license'.
| "description": "The concluded license as SPDX expression or Nonefor no license, see https://spdx.dev/spdx-specification-21-web-version#h.jxpfx0ykyb60.", | |
| "description": "The concluded license as SPDX expression or None for no license, see https://spdx.dev/spdx-specification-21-web-version#h.jxpfx0ykyb60.", |
| }, | ||
| "path": { | ||
| "default": "", | ||
| "description": "The path inside the VCS to take into account.If the VCS supports checking out only a subdirectory, only this path is checked out.", |
Copilot
AI
Nov 7, 2025
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.
Missing space after the period. Should be 'to take into account. If the VCS supports...' (note the space after the period).
| "description": "The path inside the VCS to take into account.If the VCS supports checking out only a subdirectory, only this path is checked out.", | |
| "description": "The path inside the VCS to take into account. If the VCS supports checking out only a subdirectory, only this path is checked out.", |
Initial repository configuration implementation allocated all enum and
model classes in a single file location.
Even being functional, it was diverging from Ort main Kotlin
representation, so a proper split was necessary.
Started with the curations model objects, and is the template for the subsequent iterations.