Skip to content

Conversation

@radeklat
Copy link
Owner

@radeklat radeklat commented Aug 9, 2025

No description provided.

@radeklat radeklat requested a review from Copilot August 9, 2025 14:32

This comment was marked as outdated.

Copy link

Copilot AI left a 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 introduces Delfino version 4.0.0, which includes breaking changes and modernization updates. The primary purpose is to drop support for Python 3.8 and modernize the codebase to use Python 3.9+ features, while also upgrading dependencies and tooling.

Key changes:

  • Modernize type annotations using Python 3.9+ style generics and collections
  • Add support for UV package manager detection
  • Upgrade tooling from individual tools to consolidated ruff for linting and formatting

Reviewed Changes

Copilot reviewed 31 out of 37 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pyproject.toml Updates version to 4.0.0, drops Python 3.8 support, upgrades dependencies, replaces old tooling configs with ruff
CHANGELOG.md Documents breaking changes, new features, and fixes for 4.0.0 release
src/delfino/models/pyproject_toml.py Adds BuildSystem and Project models, UV support, modernizes type annotations
src/delfino/utils.py Implements UV package manager detection logic
tests/unit/test_utils.py Adds comprehensive test coverage for package manager detection
Multiple source files Modernizes type annotations from typing module to built-in generics
Multiple test files Updates type annotations and improves code formatting
.pylintrc & tests/.pylintrc Removes pylint configuration files (replaced by ruff)
.pre-commit-config.yaml Updates formatting command from 'format' to 'ruff'
Files not reviewed (4)
  • .idea/delfino.iml: Language not supported
  • .idea/misc.xml: Language not supported
  • .idea/ruff.xml: Language not supported
  • .idea/vcs.xml: Language not supported


command_name: str = ctx.command.name or ""
value_from_config: str = getattr(app_context.plugin_config, command_name, {}).get(self.config_option_name, None)
value_from_config: str = getattr(app_context.plugin_config, command_name, {}).get(self.config_option_name, "")
Copy link

Copilot AI Aug 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default value was changed from None to an empty string, but the type annotation still indicates str. This could cause issues if the calling code expects None when no config value is found. Consider keeping None as the default or updating the type annotation to Optional[str].

Suggested change
value_from_config: str = getattr(app_context.plugin_config, command_name, {}).get(self.config_option_name, "")
value_from_config: Optional[str] = getattr(app_context.plugin_config, command_name, {}).get(self.config_option_name, None)

Copilot uses AI. Check for mistakes.
@radeklat radeklat force-pushed the release/4.0.0 branch 6 times, most recently from 3b34584 to 16f1bef Compare August 11, 2025 15:00
@codecov
Copy link

codecov bot commented Aug 11, 2025

Codecov Report

❌ Patch coverage is 65.88235% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.26%. Comparing base (22e1a4b) to head (a0a78ae).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/delfino/internal_parameters/completion.py 0.00% 10 Missing ⚠️
src/delfino/click_utils/command_groups.py 0.00% 5 Missing ⚠️
src/delfino/main.py 0.00% 4 Missing ⚠️
src/delfino/validation.py 0.00% 4 Missing ⚠️
src/delfino/utils.py 72.72% 1 Missing and 2 partials ⚠️
src/delfino/click_utils/command.py 93.33% 0 Missing and 1 partial ⚠️
src/delfino/decorators/pass_app_context.py 50.00% 1 Missing ⚠️
src/delfino/terminal_output.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #80      +/-   ##
==========================================
+ Coverage   44.33%   46.26%   +1.92%     
==========================================
  Files          22       22              
  Lines         627      642      +15     
  Branches      118       79      -39     
==========================================
+ Hits          278      297      +19     
+ Misses        342      338       -4     
  Partials        7        7              
Flag Coverage Δ
integration_tests 41.90% <60.00%> (+0.91%) ⬆️
total 46.10% <65.88%> (+2.08%) ⬆️
unit_tests 19.31% <44.70%> (+3.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@radeklat radeklat enabled auto-merge (squash) August 11, 2025 16:32
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved because this PR was opened by the repository owner and auto-merge is enabled.

@radeklat radeklat merged commit dd2bb7b into main Aug 11, 2025
10 checks passed
@radeklat radeklat deleted the release/4.0.0 branch August 11, 2025 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants