Skip to content

chore: ignore tools in codecov#3055

Merged
julienrbrt merged 1 commit intomainfrom
marko/codecov_fix
Feb 6, 2026
Merged

chore: ignore tools in codecov#3055
julienrbrt merged 1 commit intomainfrom
marko/codecov_fix

Conversation

@tac0turtle
Copy link
Contributor

@tac0turtle tac0turtle commented Feb 6, 2026

Overview

add tools to ignored codecov as its not part of the consensus critical code

@tac0turtle tac0turtle marked this pull request as ready for review February 6, 2026 08:16
@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2026

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed⏩ skipped✅ passed✅ passedFeb 6, 2026, 8:16 AM

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @tac0turtle, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refines the project's code coverage reporting by updating the codecov.yml configuration. The primary goal is to exclude the tools directory from coverage metrics, ensuring that utility or build-related code does not skew the overall coverage percentage. Concurrently, two previously ignored directories, da/internal/mocks and da/cmd, are now slated for inclusion in coverage reports, potentially broadening the scope of tested code.

Highlights

  • Codecov Configuration Update: The codecov.yml file has been modified to adjust which directories are ignored during code coverage analysis.
  • Ignoring 'tools' Directory: The tools directory is now explicitly added to the ignore list in codecov.yml, meaning its contents will be excluded from coverage reports.
  • Coverage for 'da/internal/mocks' and 'da/cmd': The previously ignored directories da/internal/mocks and da/cmd have been removed from the ignore list. This change implies that code within these directories will now be included in code coverage calculations, unless covered by other ignore rules.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • codecov.yml
    • Added 'tools' to the list of ignored directories for code coverage.
    • Removed 'da/internal/mocks' from the list of ignored directories.
    • Removed 'da/cmd' from the list of ignored directories.
Activity
  • No human activity has been recorded on this pull request since its creation.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@claude
Copy link
Contributor

claude bot commented Feb 6, 2026

Claude finished @tac0turtle's task in 1m 27s —— View job


Code Review Complete ✅

Summary

This PR updates the codecov configuration to exclude the tools directory from code coverage reporting. The change is correct and makes sense for this codebase.


Review Findings

Correct Change

The addition of tools to the ignore list is appropriate:

  • The tools/ directory contains utility programs like da-debug, blob-decoder, cache-analyzer, local-da, and db-bench
  • These are standalone CLI tools for debugging and development, not core library code
  • They don't require code coverage metrics as they're not part of the production runtime

⚠️ Question: Removal of Previous Ignore Entries

The PR removes these two entries from the ignore list:

  • da/internal/mocks
  • da/cmd

Finding: These directories don't currently exist in the codebase:

  • No files found matching da/internal/mocks/**
  • No files found matching da/cmd/**

This appears to be cleanup of stale ignore patterns. However, the PR description is empty and doesn't explain this removal. Recommendation: Add a brief explanation in the PR description mentioning that these are cleanup of non-existent directories.


Configuration Best Practices Assessment

Strengths:

  1. ✅ The ignore list is well-organized and documented with inline comments where needed
  2. ✅ Appropriate exclusions for generated code (types/pb), test directories, examples, and metrics
  3. ✅ The informational: true setting is appropriate for a project in active development

Observations:

  • The pattern block/internal/**/*_mock.go uses a specific glob for mocks
  • The new tools entry uses a simple directory pattern (consistent with apps, test, client)
  • Both approaches are valid, though the existing pkg/da/jsonrpc/mocks/ directory is not explicitly ignored (it may be covered by other rules or intentionally included)

Minor Suggestions

  1. PR Description: Consider adding a brief description explaining:

    • Why tools is being excluded from coverage
    • That da/internal/mocks and da/cmd removals are cleanup of non-existent paths
  2. Future Consideration: If mock files should be universally excluded from coverage, consider adding a broader pattern like:

    - "**/mocks/**"

    However, this is not critical for this PR since the current approach works fine.


Verdict

✅ APPROVED - This is a clean, straightforward configuration change that:

  • Correctly excludes utility tools from coverage metrics
  • Removes stale ignore patterns for non-existent directories
  • Maintains consistency with existing patterns

The only improvement would be adding context to the PR description, but the change itself is sound.


Tasks Completed

  • Read codecov.yml file to understand the changes
  • Analyze the configuration change for correctness
  • Verify the intent matches the implementation
  • Provide comprehensive review feedback

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the Codecov configuration to ignore the tools directory. However, it also removes da/internal/mocks and da/cmd from the ignore list. My review includes a suggestion to re-add these paths, as removing them could unintentionally start including mocks and command-line tool source files in coverage calculations, potentially skewing the metrics. It seems likely this was an accidental side-effect of the change.

I am having trouble creating individual review comments. Click here to see my feedback.

codecov.yml (15-16)

medium

This change replaces da/internal/mocks and da/cmd with tools. While adding tools to the ignore list is the goal of this PR, removing the other two paths might be unintentional. This could negatively affect your coverage metrics, as mocks and cmd directories are often excluded from coverage analysis for good reason. Consider keeping all three paths in the ignore list.

  - "da/internal/mocks"
  - "da/cmd"
  - "tools"

@tac0turtle tac0turtle requested a review from pthmas February 6, 2026 08:18
@codecov
Copy link

codecov bot commented Feb 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.64%. Comparing base (aedd573) to head (4d71a21).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3055      +/-   ##
==========================================
+ Coverage   56.59%   61.64%   +5.04%     
==========================================
  Files         118      111       -7     
  Lines       12152    11120    -1032     
==========================================
- Hits         6878     6855      -23     
+ Misses       4533     3526    -1007     
+ Partials      741      739       -2     
Flag Coverage Δ
combined 61.64% <ø> (+5.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.

@julienrbrt julienrbrt enabled auto-merge February 6, 2026 08:30
@julienrbrt julienrbrt added this pull request to the merge queue Feb 6, 2026
Merged via the queue into main with commit c2afe0c Feb 6, 2026
35 checks passed
@julienrbrt julienrbrt deleted the marko/codecov_fix branch February 6, 2026 08:48
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