Skip to content

Conversation

@shenxianpeng
Copy link
Collaborator

@shenxianpeng shenxianpeng commented Oct 31, 2025

Fix this line of code, to include it to support cpp-linter/clang-tools-pip#133

pyproject_path = Path(__file__).parent.parent / "pyproject.toml"

Summary by CodeRabbit

  • Chores
    • Updated PR workflow triggers for more reliable pull-request event handling.
    • Enhanced packaging metadata to include package data during installation.
  • Refactor
    • Made the tool-install resolution API public and updated internal callers to use the public API, improving reuse and clarity.

@github-actions github-actions bot added the bug Something isn't working label Oct 31, 2025
@codecov
Copy link

codecov bot commented Oct 31, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.44%. Comparing base (8d893fa) to head (604c752).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #138   +/-   ##
=======================================
  Coverage   94.44%   94.44%           
=======================================
  Files           4        4           
  Lines         108      108           
=======================================
  Hits          102      102           
  Misses          6        6           

☔ 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.

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 31, 2025

CodSpeed Performance Report

Merging #138 will not alter performance

Comparing add-include-package-data (604c752) with main (8d893fa)

Summary

✅ 60 untouched
⏩ 13 skipped1

Footnotes

  1. 13 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.

@shenxianpeng shenxianpeng changed the title fix(pyproject.toml): add include-package-data to true fix: add include-package-data to true Oct 31, 2025
@shenxianpeng shenxianpeng marked this pull request as ready for review November 1, 2025 00:08
@coderabbitai
Copy link

coderabbitai bot commented Nov 1, 2025

Walkthrough

Renames an internal install helper to a public API and updates its callers and tests; adds package-data to pyproject.toml; and narrows GitHub Actions pull_request triggers to explicit event types.

Changes

Cohort / File(s) Change Summary
GitHub Actions Workflow
\.github/workflows/pre-commit\.yml
Adds explicit pull_request trigger types: [opened, synchronize, reopened, edited].
Packaging metadata
pyproject\.toml
Sets include-package-data = true under [tool.setuptools] and adds [tool.setuptools.package-data] with cpp_linter_hooks = ["../pyproject.toml"].
Resolve-install API & callers
cpp_linter_hooks/util.py, cpp_linter_hooks/clang_format.py, cpp_linter_hooks/clang_tidy.py, tests/test_util.py
Renames _resolve_install(...)resolve_install(...) (exports now public); updates imports and all call sites and tests to use resolve_install. No other logic changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review should focus on API visibility change and its impact on exports and public surface.
  • Check util.py docstring/typing and ensure no accidental behavioral changes when making the function public.
  • Verify tests updated correctly and packaging metadata path is intended.

Possibly related PRs

Suggested labels

developer

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%.
Title Check ✅ Passed The PR title "fix: change resolve_install to public and include package data" accurately captures the two primary changes in the changeset: the renaming of the _resolve_install function to the public resolve_install function across multiple files and the addition of include-package-data = true to the pyproject.toml configuration. The title is concise, uses specific and clear language, and directly relates to the actual file modifications without being vague or misleading. Both changes are interconnected as packaging/distribution improvements and are appropriately represented in the title without excessive detail.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add-include-package-data

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.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 1, 2025

@shenxianpeng shenxianpeng changed the title fix: add include-package-data to true fix: change resolve_install to public and incude package data Nov 1, 2025
@shenxianpeng
Copy link
Collaborator Author

@coderabbitai generate docstrings

@coderabbitai
Copy link

coderabbitai bot commented Nov 1, 2025

✅ Actions performed

Initiated docstring generation; will generate only if new commits exist.

@coderabbitai
Copy link

coderabbitai bot commented Nov 1, 2025

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #139

@shenxianpeng shenxianpeng changed the title fix: change resolve_install to public and incude package data fix: change resolve_install to public and include package data Nov 1, 2025
coderabbitai bot added a commit that referenced this pull request Nov 1, 2025
Docstrings generation was requested by @shenxianpeng.

* #138 (comment)

The following files were modified:

* `cpp_linter_hooks/clang_format.py`
* `cpp_linter_hooks/clang_tidy.py`
* `cpp_linter_hooks/util.py`
@shenxianpeng shenxianpeng merged commit 583e9c3 into main Nov 1, 2025
23 of 24 checks passed
@shenxianpeng shenxianpeng deleted the add-include-package-data branch November 1, 2025 00:19
@shenxianpeng shenxianpeng changed the title fix: change resolve_install to public and include package data feat: change resolve_install to public and include package data Nov 1, 2025
@shenxianpeng shenxianpeng added developer Changes that impact developers and removed bug Something isn't working labels Nov 1, 2025
@shenxianpeng shenxianpeng changed the title feat: change resolve_install to public and include package data feat: change resolve_install to public and include package data Nov 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

developer Changes that impact developers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants