refactor(repoconfig): Get rid of legacy classes by heliocastro · Pull Request #18 · heliocastro/python-ort

heliocastro · 2026-02-20T13:22:59Z

No description provided.

Signed-off-by: Helio Chissini de Castro <dev@heliocastro.info>

Copilot

Pull request overview

This PR refactors the repository configuration code by removing legacy classes from src/ort/models/repository_configuration.py and reorganizing them into a cleaner, more modular structure under src/ort/models/config/. The refactoring replaces verbose class names (e.g., OrtRepositoryConfigurationIncludesPath) with simpler, clearer names (e.g., PathInclude), changes enums from using auto() to explicit IntEnum values, and introduces a reusable convert_enum utility function for string-to-enum field validation.

Changes:

Removed the monolithic repository_configuration.py file containing 305 lines of legacy classes and replaced it with a modular config structure
Introduced new model classes with cleaner names and explicit IntEnum values for all configuration enums (path includes/excludes, scopes, resolutions, snippets, licenses)
Added a convert_enum utility function and applied it to Issue severity validation, with plans to use it across other enum fields

Reviewed changes

Copilot reviewed 31 out of 32 changed files in this pull request and generated 17 comments.

Show a summary per file

File	Description
`src/ort/models/repository_configuration.py`	Deleted legacy file with monolithic class definitions
`src/ort/models/config/repository_configuration.py`	New modular RepositoryConfiguration with cleaner imports
`src/ort/models/config/includes.py`	New Includes model for path inclusions
`src/ort/models/config/excludes.py`	New Excludes model for path and scope exclusions
`src/ort/models/config/path_include.py`	PathInclude model with enum validator
`src/ort/models/config/path_include_reason.py`	PathIncludeReason enum changed to IntEnum with explicit values
`src/ort/models/config/path_exclude.py`	PathExclude model with enum validator added
`src/ort/models/config/path_exclude_reason.py`	PathExcludeReason enum refactored from Enum with auto() to IntEnum
`src/ort/models/config/scope_exclude.py`	New ScopeExclude model with enum validator
`src/ort/models/config/scope_exclude_reason.py`	New ScopeExcludeReason IntEnum
`src/ort/models/config/resolutions.py`	New Resolutions model aggregating issue, rule violation, and vulnerability resolutions
`src/ort/models/config/issue_resolution.py`	New IssueResolution model
`src/ort/models/config/issue_resolution_reason.py`	New IssueResolutionReason IntEnum
`src/ort/models/config/rule_violation_resolution.py`	New RuleViolationResolution model with enum validator
`src/ort/models/config/rule_violation_reason.py`	New RuleViolationResolutionReason IntEnum
`src/ort/models/config/vulnerability_resolution.py`	New VulnerabilityResolution model
`src/ort/models/config/vulnerability_resolution_reason.py`	New VulnerabilityResolutionReason IntEnum
`src/ort/models/config/license_choice.py`	New LicenseChoice and PackageLicenseChoice models
`src/ort/models/config/snippet/snippet_choice.py`	New SnippetChoice, Given, and Choice models for snippet selection
`src/ort/models/config/snippet/snippet_choice_reason.py`	New SnippetChoiceReason IntEnum
`src/ort/models/config/snippet/Provenance.py`	New Provenance model for repository provenance URLs
`src/ort/models/text_location.py`	New TextLocation model for file location references
`src/ort/utils/spdx/spdx_license_choice.py`	New SpdxLicenseChoice model for license selections
`src/ort/utils/spdx/spdx_expression.py`	New SpdxExpression IntEnum for validation strictness levels
`src/ort/utils/convert_enum.py`	New utility function for string-to-enum conversion
`src/ort/utils/__init__.py`	Exported convert_enum utility
`src/ort/models/__init__.py`	Updated exports to include new config models
`src/ort/__init__.py`	Updated import path for RepositoryConfiguration
`src/ort/models/repository.py`	Updated import path for RepositoryConfiguration
`src/ort/models/issue.py`	Refactored to use convert_enum utility
`tests/test_repository_configuration.py`	Updated to use new class names and enum values

Comments suppressed due to low confidence (1)

tests/test_repository_configuration.py:65

The error message in the docstring refers to "RepositoryConfigurationIncludesPath" which is the old class name from the legacy code. It should be updated to "PathInclude" to match the new class name being used in the refactored code.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-20T13:29:12Z

src/ort/models/config/path_include.py

+    Defines paths which should be excluded. Each file or directory that is matched by the [glob][pattern] is marked as
+    excluded. If a project definition file is matched by the [pattern], the whole project is excluded. For details about


The docstring says "Defines paths which should be excluded" but this is the PathInclude class that defines paths to be included. The docstring should say "Defines paths which should be included." Additionally, the rest of the description talks about exclusions ("marked as excluded", "whole project is excluded") which is incorrect for an include operation.

Suggested change

Defines paths which should be excluded. Each file or directory that is matched by the [glob][pattern] is marked as

excluded. If a project definition file is matched by the [pattern], the whole project is excluded. For details about

Defines paths which should be included. Each file or directory that is matched by the [glob][pattern] is marked as

included. If a project definition file is matched by the [pattern], the whole project is included. For details about

Copilot · 2026-02-20T13:29:13Z

src/ort/models/config/license_choice.py

+    repository_license_choices: list[SpdxLicenseChoice] = Field(
+        default_factory=list,
+        description="SPDX",
+    )


The description is incomplete and unhelpful. It just says "SPDX" but should provide a meaningful description of what this field represents, such as "SPDX license choices applied to all packages in the repository."

Copilot · 2026-02-20T13:29:13Z

src/ort/models/config/snippet/snippet_choice.py

+    purl: str = Field(
+        ...,
+        description="The purl of the snippet chosen by this snippet choice."


The 'purl' field is marked as required (using ...) but the description states "If [reason] is [SnippetChoiceReason.NO_RELEVANT_FINDING], it is null." This is inconsistent. If the field can be null, it should be defined as 'purl: str | None' with a default of None, not as required.

Suggested change

purl: str = Field(

...,

description="The purl of the snippet chosen by this snippet choice."

purl: str | None = Field(

None,

description="The purl of the snippet chosen by this snippet choice. "

Copilot · 2026-02-20T13:29:13Z

src/ort/models/config/snippet/snippet_choice.py

+    )
+    purl: str = Field(
+        ...,
+        description="The purl of the snippet chosen by this snippet choice."


Missing a space between sentences in the description. It should read "snippet choice. If [reason]" instead of "snippet choice.If [reason]"

Suggested change

description="The purl of the snippet chosen by this snippet choice."

description="The purl of the snippet chosen by this snippet choice. "

Copilot · 2026-02-20T13:29:14Z

src/ort/models/config/license_choice.py

+
+class PackageLicenseChoice(BaseModel):
+    """
+    SpdxLicenseChoice]s defined for an artifact.


The docstring has a missing opening bracket. It says "SpdxLicenseChoice]s defined" but should say "[SpdxLicenseChoice]s defined" to be consistent with the documentation style used elsewhere.

Suggested change

SpdxLicenseChoice]s defined for an artifact.

[SpdxLicenseChoice]s defined for an artifact.

Copilot · 2026-02-20T13:29:16Z

src/ort/models/config/issue_resolution.py

+    reason: IssueResolutionReason = Field(
+        description="The reason why the issue is resolved.",
+    )


The IssueResolution class has an enum field 'reason' of type IssueResolutionReason but doesn't have a field_validator to convert string values to enum values. This is inconsistent with similar classes in the codebase like PathExclude, PathInclude, ScopeExclude, and RuleViolationResolution which all use the convert_enum utility with a field_validator. Consider adding a validator for consistency and to properly handle string-to-enum conversion.

Copilot · 2026-02-20T13:29:16Z

src/ort/models/config/includes.py

+
+class Includes(BaseModel):
+    """
+    Defines which parts of a repository should be excluded.


The docstring says "Defines which parts of a repository should be excluded" but this is the Includes class that defines which parts should be included. The docstring should say "Defines which parts of a repository should be included."

Suggested change

Defines which parts of a repository should be excluded.

Defines which parts of a repository should be included.

Copilot · 2026-02-20T13:29:17Z

src/ort/utils/spdx/spdx_license_choice.py

+    given: str | None = Field(
+        default=None,
+        description="SPDX",
+    )


The description is incomplete and unhelpful. It just says "SPDX" but should provide a meaningful description of what this field represents, such as "The SPDX expression to match" or "The given SPDX license expression or sub-expression."

Copilot · 2026-02-20T13:29:17Z

src/ort/models/config/rule_violation_resolution.py

+
+    message: str = Field(
+        description="A regular expression string to match the messages of rule violations to resolve."
+        "Whitespace in the message will be [collapsed][collapseWhitespace] and it will be converted to"


Missing a space between sentences. The description should have a space after the period: "to resolve. Whitespace in the message"

Suggested change

"Whitespace in the message will be [collapsed][collapseWhitespace] and it will be converted to"

" Whitespace in the message will be [collapsed][collapseWhitespace] and it will be converted to"

Copilot · 2026-02-20T13:29:17Z

src/ort/models/config/snippet/snippet_choice.py

+    reason: SnippetChoiceReason = Field(
+        ...,
+        description="The reason why this snippet choice was made.",
+    )


The Choice class has an enum field 'reason' of type SnippetChoiceReason but doesn't have a field_validator to convert string values to enum values. This is inconsistent with similar classes in the codebase like PathExclude, PathInclude, ScopeExclude, and RuleViolationResolution which all use the convert_enum utility with a field_validator. Consider adding a validator for consistency and to properly handle string-to-enum conversion.

refactor(repoconfig): Get rid of legacy classes

8a6eae6

Signed-off-by: Helio Chissini de Castro <dev@heliocastro.info>

Copilot AI review requested due to automatic review settings February 20, 2026 13:23

heliocastro self-assigned this Feb 20, 2026

Copilot started reviewing on behalf of heliocastro February 20, 2026 13:23 View session

heliocastro merged commit 39e4ffb into main Feb 20, 2026
20 checks passed

heliocastro deleted the refactor/repository_config branch February 20, 2026 13:28

Copilot AI reviewed Feb 20, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(repoconfig): Get rid of legacy classes#18

refactor(repoconfig): Get rid of legacy classes#18
heliocastro merged 1 commit intomainfrom
refactor/repository_config

heliocastro commented Feb 20, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		Defines paths which should be excluded. Each file or directory that is matched by the [glob][pattern] is marked as
		excluded. If a project definition file is matched by the [pattern], the whole project is excluded. For details about

	description="The purl of the snippet chosen by this snippet choice."
	description="The purl of the snippet chosen by this snippet choice. "

	SpdxLicenseChoice]s defined for an artifact.
	[SpdxLicenseChoice]s defined for an artifact.

	Defines which parts of a repository should be excluded.
	Defines which parts of a repository should be included.

	"Whitespace in the message will be [collapsed][collapseWhitespace] and it will be converted to"
	" Whitespace in the message will be [collapsed][collapseWhitespace] and it will be converted to"

Conversation

heliocastro commented Feb 20, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants