-
Notifications
You must be signed in to change notification settings - Fork 176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(CI): remove CI
env var to fix the CI
#1704
Conversation
xref: materialsproject/pymatgen#4243 (comment) Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
📝 WalkthroughWalkthroughThe pull request modifies the GitHub Actions workflow by replacing the command to run tests in the "Test" step with a multi-line script. This new script first unsets the Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
26-26
: Fix the malformed issue link in the commentThe issue link contains an extra '1' at the end (4243#issuecomment-2573310051), which makes it invalid.
- # https://github.com/materialsproject/pymatgen/issues/4243#issuecomment-2573310051 + # https://github.com/materialsproject/pymatgen/issues/4243#issuecomment-257331005
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yml
(1 hunks)
🔇 Additional comments (1)
.github/workflows/test.yml (1)
25-27
: Verify the impact of unsetting the CI environment variableWhile this change addresses the pymatgen issue, setting
CI=""
might affect other tools that rely on CI environment detection. Let's verify this is the recommended approach and document any potential side effects.Consider adding a comment explaining:
- What specific issue this fixes in pymatgen
- Any known side effects on other tools
- Whether this is a temporary workaround or permanent solution
✅ Verification successful
The CI environment variable change appears safe to implement
The codebase analysis shows no usage of the CI environment variable in the repository or its configuration files. The change specifically addresses the pymatgen issue #4243 and shouldn't affect other tools.
Consider expanding the comment to:
env: # Workaround for pymatgen issue causing test failures in CI # See: https://github.com/materialsproject/pymatgen/issues/4243#issuecomment-2573310051 CI: ""🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for CI environment variable usage in dependencies rg -l "os\.environ\.get\(['\"]CI['\"]" || rg -l "os\.getenv\(['\"]CI['\"]" # Check if any GitHub Actions specific code exists that might be affected rg -l "GITHUB_ACTIONS.*CI"Length of output: 112
Script:
#!/bin/bash # Search for any CI environment variable references rg -l "CI['\"]?\s*[:=]" # Search for CI environment checks in various forms rg -l "process\.env\.CI|env\(['\"]*CI|CI\s*=|CI:|\"CI\":|'CI':" # Check package.json files for CI-related configurations fd package.json --exec cat {} \; | jq -r 'select(. | tostring | contains("CI"))' # Look for GitHub Actions specific CI configurations rg -l "env:.*CI|CI:.*\$\{\{|CI=.*\$\{\{"Length of output: 226
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #1704 +/- ##
=======================================
Coverage 49.51% 49.51%
=======================================
Files 83 83
Lines 14863 14863
=======================================
Hits 7359 7359
Misses 7504 7504 ☔ View full report in Codecov by Sentry. |
Reverts #1704 See materialsproject/pymatgen#4243 (comment) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Chores** - Updated GitHub Actions workflow configuration for testing. - Simplified test execution process in the CI pipeline. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
xref: materialsproject/pymatgen#4243 (comment)
Summary by CodeRabbit