Skip to content

Conversation

@MSevey
Copy link
Contributor

@MSevey MSevey commented Nov 3, 2023

Overview

Since the third_party directory is just copy pasted code from other repos, primarily to avoid circular dependencies, we can ignore it in the testing and coverage reports since we don't want to make any changes here. All changes should be upstreamed and then re-copied in.

Summary by CodeRabbit

  • Tests
    • Introduced an option to ignore specific directories during testing and linting for more flexible configuration.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2023

Walkthrough

The introduced changes in the Makefile provide a new feature that allows for ignoring specific directories during the testing and linting process. This is achieved by introducing a new variable IGNORE_DIRS and modifying the pkgs variable to exclude directories specified in IGNORE_DIRS.

Changes

File Summary
.../Makefile Introduced a new variable IGNORE_DIRS to specify directories to ignore during testing and linting. Modified pkgs variable to exclude directories in IGNORE_DIRS using a grep command.

🐇🍂

As the leaves fall and the wind blows,

In the world of code, a new feature grows.

Ignore the noise, focus on the core,

With IGNORE_DIRS, testing's a chore no more.

As we hop through the autumn's golden hue,

Here's to the coder, hats off to you! 🎩🍁

So let's celebrate this day, let's raise a cheer,

For the beauty of code, so crystal clear. 🥂🎉


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai help to get help.
  • @coderabbitai resolve to resolve all the CodeRabbit review comments.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.json

@MSevey MSevey added T:testing Related to testing C:ci Component: Continuous Integration labels Nov 3, 2023
@MSevey MSevey requested review from Manav-Aggarwal and removed request for nashqueue November 3, 2023 09:38
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between f133b08 and 30791d2.
Files selected for processing (1)
  • Makefile (1 hunks)
Additional comments: 1
Makefile (1)
  • 3-13: The changes to the Makefile look good. The IGNORE_DIRS variable is a good addition for excluding certain directories from the testing and linting process. The use of grep -vE to exclude directories specified in IGNORE_DIRS from the pkgs variable is a good approach. Ensure that the IGNORE_DIRS variable is correctly set in the environment where the CI process runs.

@codecov
Copy link

codecov bot commented Nov 3, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f133b08) 53.64% compared to head (30791d2) 52.63%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1310      +/-   ##
==========================================
- Coverage   53.64%   52.63%   -1.02%     
==========================================
  Files          64       52      -12     
  Lines        7042     6222     -820     
==========================================
- Hits         3778     3275     -503     
+ Misses       2905     2635     -270     
+ Partials      359      312      -47     

see 13 files with indirect coverage changes

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

Copy link
Member

@nashqueue nashqueue left a comment

Choose a reason for hiding this comment

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

utAck

@nashqueue nashqueue added this pull request to the merge queue Nov 3, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 3, 2023
@nashqueue nashqueue added this pull request to the merge queue Nov 3, 2023
Merged via the queue into main with commit cb1052a Nov 3, 2023
@nashqueue nashqueue deleted the sevey/ignore-third-party branch November 3, 2023 14:14
gupadhyaya pushed a commit that referenced this pull request Nov 8, 2023
<!--
Please read and fill out this form before submitting your PR.

Please make sure you have reviewed our contributors guide before
submitting your
first PR.
-->

## Overview

Since the `third_party` directory is just copy pasted code from other
repos, primarily to avoid circular dependencies, we can ignore it in the
testing and coverage reports since we don't want to make any changes
here. All changes should be upstreamed and then re-copied in.


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

## Summary by CodeRabbit

- **Tests**
- Introduced an option to ignore specific directories during testing and
linting for more flexible configuration.

<!-- 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

C:ci Component: Continuous Integration T:testing Related to testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants