Skip to content

feat: add option to write summary to file#206

Merged
2bndy5 merged 8 commits into
cpp-linter:mainfrom
philippkarg:feat/add-summary-output-path
Jun 26, 2026
Merged

feat: add option to write summary to file#206
2bndy5 merged 8 commits into
cpp-linter:mainfrom
philippkarg:feat/add-summary-output-path

Conversation

@philippkarg

@philippkarg philippkarg commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Closes #205.

  • Use option -o or --summary-output-file to provide a file.
  • By default the summary will not be written to a file.
  • If the provided path is a directory, log an error.

Summary by CodeRabbit

  • New Features

    • Added -o/--summary-output-file to write the generated step summary to a specified Markdown file path (use an empty value to disable file output).
  • Bug Fixes

    • Feedback summaries are now produced when built-in step summaries are enabled and/or when a summary output file is provided.
    • When using a summary output file, missing parent directories are created and write errors are handled without crashing.
  • Tests

    • Expanded CLI argument parsing and feedback/comment tests to cover summary file creation and edge cases.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The PR adds a --summary-output-file CLI option, stores it on Args, and updates GithubApiClient.post_feedback to write generated summary text to GITHUB_STEP_SUMMARY and an optional file path. Tests cover parsing and file-writing behavior.

Changes

Summary output file support

Layer / File(s) Summary
CLI option and parser tests
cpp_linter/cli.py, tests/test_cli_args.py
Args defines summary_output_file, _parser_args accepts -o/--summary-output-file, and parser tests cover a filename value and an empty value.
Summary output writing
cpp_linter/rest_api/github_api.py, tests/comments/test_comments.py
post_feedback computes whether to write the step summary, appends the generated comment to GITHUB_STEP_SUMMARY, writes the same comment to args.summary_output_file, and tests cover writable-file and directory-path cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding an option to write the summary to a file.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp_linter/rest_api/github_api.py`:
- Around line 242-251: The optional summary file write in the `post_feedback()`
flow can raise an `OSError` and abort annotations when
`args.summary_output_file` is invalid or its parent directory is missing. Wrap
the `summary_output_path.open(...)` write path in a best-effort `try/except
OSError` inside the `summary_output_path` handling block, and log the failure
with `log_commander.error` while allowing the rest of `post_feedback()` to
continue. Keep the existing `Path` and `summary_output_path.is_dir()` checks,
and ensure only the extra file output is skipped on failure.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3a646e43-3e67-48fd-8775-ba7ae53eba84

📥 Commits

Reviewing files that changed from the base of the PR and between c68104c and 670dfed.

📒 Files selected for processing (4)
  • cpp_linter/cli.py
  • cpp_linter/rest_api/github_api.py
  • tests/comments/test_comments.py
  • tests/test_cli_args.py

Comment thread cpp_linter/rest_api/github_api.py Outdated
@philippkarg philippkarg marked this pull request as draft June 26, 2026 13:59

@2bndy5 2bndy5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The test CI seems to be failing for a separate reason. @shenxianpeng Can you update the CI for the breaking CLI changes in clang-tools-pip v1?

Comment thread cpp_linter/cli.py Outdated
Comment thread cpp_linter/rest_api/github_api.py Outdated
Comment thread cpp_linter/rest_api/github_api.py Fixed
@philippkarg philippkarg marked this pull request as ready for review June 26, 2026 15:05
@philippkarg philippkarg changed the title feat: Add option to write summary to file. feat: Add option to write summary to file Jun 26, 2026
@philippkarg philippkarg changed the title feat: Add option to write summary to file feat: add option to write summary to file Jun 26, 2026

@2bndy5 2bndy5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One last tweak.

I'll have to fix the test CI in a separate PR and rebase this branch when that's merged.

Comment thread cpp_linter/rest_api/github_api.py Outdated
Philipp Karg and others added 5 commits June 26, 2026 21:25
- Use option `-o` or `--summary-output-file` to provide a file.
- By default the summary will not be written to a file.
- If the provided path is a directory, log an error.
Co-authored-by: Brendan <2bndy5@gmail.com>
- Catch OSErrors when writing the summary output file.}
- Log the errors, but don't stop the workflow.
@shenxianpeng shenxianpeng force-pushed the feat/add-summary-output-path branch from c42596c to 7ee169f Compare June 26, 2026 19:25
@shenxianpeng

Copy link
Copy Markdown
Member

The test CI seems to be failing for a separate reason. @shenxianpeng Can you update the CI for the breaking CLI changes in clang-tools-pip v1?

Just see your comment right now after the CI failure was fixed 😅

@philippkarg

Copy link
Copy Markdown
Contributor Author

Ahh I fucked up the import sorting in my last commit. Let me fix that

@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.31%. Comparing base (eaaa671) to head (cabd3a2).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #206      +/-   ##
==========================================
+ Coverage   98.29%   98.31%   +0.01%     
==========================================
  Files          24       24              
  Lines        1938     1958      +20     
==========================================
+ Hits         1905     1925      +20     
  Misses         33       33              

☔ View full report in Codecov by Harness.
📢 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.

also updates the --help output about the `--summary-output-file`

@2bndy5 2bndy5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great work team!

I added some polish to --help output about relative paths and catching encoding errors (with ValueError)

@2bndy5

2bndy5 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

New CLI doc looks good too: https://cpp-linter--206.org.readthedocs.build/en/206/cli_args.html#cmdoption-o

@2bndy5 2bndy5 temporarily deployed to test-coverage June 26, 2026 20:22 — with GitHub Actions Inactive
@2bndy5 2bndy5 merged commit e8981d1 into cpp-linter:main Jun 26, 2026
39 checks passed
@2bndy5 2bndy5 added enhancement New feature or request minor A minor version bump labels Jun 26, 2026
2bndy5 added a commit to cpp-linter/cpp-linter-rs that referenced this pull request Jun 27, 2026
ref
- cpp-linter/cpp-linter#205
- cpp-linter/cpp-linter#206

Allows exporting the step summary comment to a custom file path.

Useful as a workaround for contexts with restricted permissions (eg
private repos).

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
  * Added a new CLI option to save the generated summary to a file.
* Summary files can use either an absolute path or a path relative to
the repository root.

* **Bug Fixes**
* Summary content is now generated whenever file output is requested,
even if step-based summary posting is off.
  * Improved error reporting for file and directory write failures.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request minor A minor version bump

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants