-
Notifications
You must be signed in to change notification settings - Fork 1
feat(package_config): Add classes for config package_configurations #13
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
feat(package_config): Add classes for config package_configurations #13
Conversation
heliocastro
commented
Nov 10, 2025
- Removed old tests pointing to old models
- Added validators to proper parse Identifier
- Removed old tests pointing to old models - Added validators to proper parse Identifier 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 from 0.3.1 to 0.4.0 and performs a significant refactoring of the ORT configuration model architecture. The changes reorganize package manager and analyzer configuration code into a more modular structure under the config module.
Key Changes:
- Refactored configuration models into dedicated config module with new classes for
PackageConfiguration,RepositoryAnalyzerConfiguration,PackageManagerConfiguration, and related types - Removed deprecated
OrtPackageManagerConfigurationsandOrtAnalyzerConfigurationsclasses - Added support for flexible
start_linesparsing inLicenseFindingCuration(string, list, or int) - Updated test error messages to correctly reference
Curationsinstead ofOrtRepositoryConfiguration
Reviewed Changes
Copilot reviewed 22 out of 24 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Version bump from 0.3.1 to 0.4.0 |
| pyproject.toml | Version bump from 0.3.1 to 0.4.0 |
| tests/test_package_curation.py | Fixed error messages to correctly reference Curations class |
| tests/test_package_configuration.py | New test file for validating package configuration loading |
| tests/data/example_simple_package_config.yml | New test data file with example package configuration |
| tests/test_ort_configuration.py | Removed deprecated test file |
| src/ort/init.py | Removed exports of deprecated configuration classes |
| src/ort/models/repository_configuration.py | Updated imports to use new config module classes |
| src/ort/models/ort_configuration.py | Commented out deprecated package manager fields |
| src/ort/models/package_managers.py | Removed deprecated file |
| src/ort/models/analyzer_configurations.py | Removed deprecated file |
| src/ort/models/identifier.py | New model for package identifiers with string parsing support |
| src/ort/models/config/vcsmatcher.py | New VCS matcher model for repository provenance matching |
| src/ort/models/config/repository_analyzer_configuration.py | New repository-level analyzer configuration model |
| src/ort/models/config/path_exclude_reason.py | Placeholder file with only license headers |
| src/ort/models/config/path_exclude.py | New model for path exclusion patterns |
| src/ort/models/config/package_manager_configuration.py | New package manager configuration model |
| src/ort/models/config/package_configuration.py | New package configuration model |
| src/ort/models/config/license_finding_curation.py | Added flexible start_lines field validator |
| src/ort/models/config/analyzer_configuration.py | New global analyzer configuration model |
| src/ort/models/package_curation_data.py | Added comprehensive docstring |
| schemas/repository-configuration-schema.json | Updated JSON schema to reflect VcsInfoCurationData changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
| package_managers: dict[str, PackageManagerConfiguration] | None = Field( | ||
| default=None, | ||
| description="Get a [PackageManagerConfiguration] from [packageManagers]. The difference to accessing the map" |
Copilot
AI
Nov 10, 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 period in description text. Should be "The difference to accessing the map directly" instead of "The difference to accessing the mapdirectly".
| description="Get a [PackageManagerConfiguration] from [packageManagers]. The difference to accessing the map" | |
| description="Get a [PackageManagerConfiguration] from [packageManagers]. The difference to accessing the map " |
| ) | ||
|
|
||
| id: Identifier = Field( | ||
| description="The [Identifier] which must match with the identifier of the package in order for this package" |
Copilot
AI
Nov 10, 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 period in description text. Should be "The [Identifier] which must match with the identifier of the package in order" instead of "The [Identifier] which must match with the identifier of the package inorder".
| description="The [Identifier] which must match with the identifier of the package in order for this package" | |
| description="The [Identifier] which must match with the identifier of the package in order for this package " |
| # enabled_package_managers: Annotated[list[PackageManager] | None, Field(alias="enabledPackageManagers")] = None | ||
| # # disabled_package_managers: Annotated[list[OrtPackageManagers] | None, | ||
| # Field(alias="disabledPackageManagers")] = None | ||
| # package_managers: Annotated[OrtPackageManagerConfigurations | None, Field(alias="packageManagers")] = None |
Copilot
AI
Nov 10, 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.
Commented-out code should be removed rather than left in the codebase. If this functionality is no longer needed, delete these lines. If it will be needed in the future, rely on version control history to retrieve it.
| # enabled_package_managers: Annotated[list[PackageManager] | None, Field(alias="enabledPackageManagers")] = None | |
| # # disabled_package_managers: Annotated[list[OrtPackageManagers] | None, | |
| # Field(alias="disabledPackageManagers")] = None | |
| # package_managers: Annotated[OrtPackageManagerConfigurations | None, Field(alias="packageManagers")] = None |
| must_run_after: list[str] | None = Field( | ||
| default=None, | ||
| description="The configuration model for a package manager. This class is (de-)serialized in the following" | ||
| "places:" | ||
| "- Deserialized from config.yml as part of [OrtConfiguration] (via Hoplite)." | ||
| "- Deserialized from .ort.yml as part of [RepositoryAnalyzerConfiguration] (via Jackson)" | ||
| "- (De-)Serialized as part of [org.ossreviewtoolkit.model.OrtResult] (via Jackson).", | ||
| ) |
Copilot
AI
Nov 10, 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 description on the must_run_after field appears to be misplaced. This description talks about the overall configuration model for package managers, but should specifically describe what must_run_after does - i.e., that it specifies package managers that must run before this one.
| ) | ||
| package_managers: dict[str, PackageManagerConfiguration] | None = Field( | ||
| default=None, | ||
| description="Get a [PackageManagerConfiguration] from [packageManagers]. The difference to accessing the map" |
Copilot
AI
Nov 10, 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 period in description text. Should be "The difference to accessing the map directly" instead of "The difference to accessing the mapdirectly".
| ) | ||
|
|
||
| namespace: str = Field( | ||
| description="The namespace of the component, for examplethe group for 'Maven' or the scope for 'NPM'.", |
Copilot
AI
Nov 10, 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 in description text. Should be "for example the group" instead of "for examplethe group".
| revision: str | None = Field( | ||
| default=None, | ||
| description="The revision to match for equality against [RepositoryProvenance.resolvedRevision]," | ||
| "or null to match anyrevision.", |
Copilot
AI
Nov 10, 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 in description text. Should be "any revision" instead of "anyrevision".
| allow_dynamic_versions: bool | None = Field( | ||
| default=None, | ||
| description="Enable the analysis of projects that use version ranges to declare their dependencies." | ||
| "If set to true, dependencies of exactly the same project might change with another scan done at a later time" |
Copilot
AI
Nov 10, 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 "." in description text. Should be "If set to true, dependencies" instead of "If set to true,dependencies".
| description="A list of the case-insensitive names of package managers that are enabled." | ||
| "Disabling a package manager in [disabledPackageManagers] overrides enabling it here.", |
Copilot
AI
Nov 10, 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 period in description text. Should be "Disabling a package manager in [disabledPackageManagers] overrides" instead of "Disabling a package manager in [disabledPackageManagers]overrides".
| description="A list of the case-insensitive names of package managers that are enabled." | ||
| "Disabling a package manager in [disabledPackageManagers] overrides enabling it here.", |
Copilot
AI
Nov 10, 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 period in description text. Should be "Disabling a package manager in [disabledPackageManagers] overrides" instead of "Disabling a package manager in [disabledPackageManagers]overrides".