Skip to content

Conversation

srilman
Copy link
Contributor

@srilman srilman commented Sep 3, 2025

Changes Made

Undo the change in the AGENTS.md that broke the check

Related Issues

Checklist

  • Documented in API Docs (if applicable)
  • Documented in User Guide (if applicable)
  • If adding a new documentation page, doc is added to docs/mkdocs.yml navigation
  • Documentation builds and is formatted properly (tag @/ccmao1130 for docs review)

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This PR fixes a broken skipcheck mechanism in the GitHub Actions workflow for the Daft repository. The change reverts problematic logic that was incorrectly trying to capture git diff output in a variable and check exit codes manually.

The fix simplifies the conditional logic by using git diff --quiet directly in an if statement, which is the standard and reliable approach for checking if there are differences between commits. The skipcheck job is a CI optimization that allows the workflow to skip expensive test suites when only non-functional files are changed (documentation, configs, etc.), which is crucial for maintaining CI efficiency in a large codebase like Daft.

The corrected logic maintains the same comprehensive file exclusion list, including .vscode/**, ci/**, docs/**, configuration files like .gitignore, codecov.yml, and documentation files like AGENTS.md, README.rst, and CONTRIBUTING.md. When changes are limited to these files, the workflow will properly skip the full test suite, saving CI resources while ensuring that functional code changes still trigger complete testing.

PR Description Notes:

  • The description mentions "Undo the change in the AGENTS.md that broke the check" but the actual change is in .github/workflows/pr-test-suite.yml, not AGENTS.md itself

Confidence score: 5/5

  • This PR is safe to merge with minimal risk as it fixes a clearly broken CI mechanism
  • Score reflects a straightforward fix to restore proper workflow functionality with well-established git diff patterns
  • No files require special attention as this is a simple revert to working logic

1 file reviewed, no comments

Edit Code Review Bot Settings | Greptile

@github-actions github-actions bot added the fix label Sep 3, 2025
Copy link
Collaborator

@desmondcheongzx desmondcheongzx left a comment

Choose a reason for hiding this comment

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

For posterity:

--exit-code
Make the program exit with codes similar to diff(1). That is, it exits with 1 if there were differences and 0 means no differences.

--quiet
Disable all output of the program. Implies --exit-code. Disables execution of external diff helpers whose exit code is not trusted, i.e. their respective configuration option diff.trustExitCode or diff.<driver>.trustExitCode or environment variable GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE is false.

@srilman srilman enabled auto-merge (squash) September 3, 2025 22:16
@srilman srilman merged commit f187716 into main Sep 4, 2025
54 checks passed
@srilman srilman deleted the slade/fix-skipcheck branch September 4, 2025 00:24
Copy link

codecov bot commented Sep 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.51%. Comparing base (81c2e1a) to head (ceab96d).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5131      +/-   ##
==========================================
- Coverage   76.54%   76.51%   -0.03%     
==========================================
  Files         953      953              
  Lines      130653   130727      +74     
==========================================
+ Hits       100006   100030      +24     
- Misses      30647    30697      +50     

see 18 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

venkateshdb pushed a commit to venkateshdb/Daft that referenced this pull request Sep 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants