Skip to content

Refactor ruff check+format to use sequential subprocess calls#2967

Merged
koxudaxi merged 1 commit intomainfrom
refactor/ruff-sequential-execution
Jan 19, 2026
Merged

Refactor ruff check+format to use sequential subprocess calls#2967
koxudaxi merged 1 commit intomainfrom
refactor/ruff-sequential-execution

Conversation

@koxudaxi
Copy link
Copy Markdown
Owner

@koxudaxi koxudaxi commented Jan 19, 2026 •
edited
Loading

Closes: #2966

Summary by CodeRabbit

  • Chores
    • Streamlined the internal code formatting and linting flow to a simpler, sequential process for more reliable and consistent formatting results.
    • Cleaned up implementation details to reduce edge-case failures and make formatting outcomes more predictable.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 19, 2026 •
edited
Loading

📝 Walkthrough

Walkthrough

The Ruff formatting pipeline in src/datamodel_code_generator/format.py was refactored to run the check and format steps sequentially using subprocess.run, passing stdout from the check step into the format step and returning the final formatted output.

Changes

Cohort / File(s) Summary
Ruff formatting pipeline refactor
src/datamodel_code_generator/format.py		 Replaced piped Popen subprocess pattern with two sequential subprocess.run calls; ruff check --fix runs first producing check\_result, then ruff format runs with check\_result.stdout as input producing format\_result; final return is format\_result.stdout (decoded)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through code in tidy rows,

From piped accord to ordered flows,
Check then format, step by step,
No tangled pipes to intercept—
A neater trail where logic grows.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: refactoring the ruff check-and-format pipeline from a combined Popen-based approach to sequential subprocess.run calls.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 19, 2026 •
edited
Loading

📚 Docs Preview: https://pr-2967.datamodel-code-generator.pages.dev

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jan 19, 2026 •
edited
Loading

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 11 untouched benchmarks
⏩ 98 skipped benchmarks1

Comparing refactor/ruff-sequential-execution (7df4681) with main (8c60b1b)

Open in CodSpeed

Footnotes

  1. 98 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 19, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (8c60b1b) to head (7df4681).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff            @@
##              main     #2967   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           94        94           
  Lines        17787     17781    -6     
  Branches      2045      2044    -1     
=========================================
- Hits         17787     17781    -6
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

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.

@koxudaxi koxudaxi marked this pull request as draft January 19, 2026 17:59
@koxudaxi koxudaxi force-pushed the refactor/ruff-sequential-execution branch from 4f46326 to 7df4681 Compare January 19, 2026 18:11
@koxudaxi koxudaxi marked this pull request as ready for review January 19, 2026 18:18
@koxudaxi koxudaxi merged commit 4661406 into main Jan 19, 2026
37 checks passed
@koxudaxi koxudaxi deleted the refactor/ruff-sequential-execution branch January 19, 2026 18:22
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 19, 2026

Breaking Change Analysis

Result: No breaking changes detected

Reasoning: This PR refactors the internal implementation of the apply_ruff_check_and_format method in CodeFormatter. The change replaces a piped Popen approach (where ruff check stdout was piped directly to ruff format stdin) with sequential subprocess.run calls. This is a purely internal implementation detail that:

  1. Does not change any public API - the method signature remains apply_ruff_check_and_format(self, code: str) -> str
  2. Does not change the generated code output - the same ruff check and format operations are performed
  3. Does not change CLI options or Python API
  4. Does not change default behavior - formatters still work the same way
  5. Does not affect custom templates
  6. Does not change error handling behavior externally

The change was made to fix edge-case failures (as mentioned in the PR description: "reduce edge-case failures and make formatting outcomes more predictable"). This is a bug fix / reliability improvement, not a breaking change.

This analysis was performed by Claude Code Action

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 14, 2026

🎉 Released in 0.54.0

This PR is now available in the latest release. See the release notes for details.

@github-actions github-actions Bot mentioned this pull request Feb 14, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

breaking-change-analyzed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@koxudaxi