Suppress animation in CI contexts Fixes #702#703
Conversation
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Fix Detected |
|---|---|---|
| Incomplete CI Environment Detection ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| dfetch/project/vcs.py | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Need a new review? Comment
/korbit-reviewon this PR and I'll review your latest changes.Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-reviewcommand in a comment at the root of your PR.- You can ask Korbit to generate a new PR description using the
/korbit-generate-pr-descriptioncommand in any comment on your PR.- Too many Korbit comments? I can resolve all my comment threads if you use the
/korbit-resolvecommand in any comment on your PR.- On any given comment that Korbit raises on your pull request, you can have a discussion with Korbit by replying to the comment.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Current Korbit Configuration
General Settings
Setting Value Review Schedule Automatic excluding drafts Max Issue Count 10 Automatic PR Descriptions ✅ Issue Categories
Category Enabled Documentation ✅ Logging ✅ Error Handling ✅ Readability ✅ Design ✅ Performance ✅ Security ✅ Functionality ✅ Feedback and Support
Note
Korbit Pro is free for open source projects 🎉
Looking to add Korbit to your team? Get started with a free 2 week trial here
| def _running_in_ci() -> bool: | ||
| """Are we running in CI.""" | ||
| ci_env_var = os.getenv('CI', None) | ||
| return ci_env_var and (ci_env_var.lower().startswith("t") or ci_env_var == "1") |
There was a problem hiding this comment.
Incomplete CI Environment Detection 
Tell me more
What is the issue?
The CI environment detection logic only accepts 'true', 't' (case-insensitive), or '1' as valid values, but misses common CI environment variations like 'TRUE', 'yes', 'Y', etc.
Why this matters
CI systems use various truthy values to set environment variables. The current implementation might fail to detect some CI environments, causing animations to run when they shouldn't be suppressed.
Suggested change ∙ Feature Preview
Expand the CI environment detection to include more common truthy values:
@staticmethod
def _running_in_ci() -> bool:
"""Are we running in CI."""
ci_env_var = os.getenv('CI', '').lower()
truthy_values = ('true', 't', '1', 'yes', 'y')
return any(ci_env_var.startswith(val) for val in truthy_values)Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
There was a problem hiding this comment.
We could also check the first char if it is t, y or 1
4aa0a22 to
7f0f495
Compare
7f0f495 to
2d2c63e
Compare
Description by Korbit AI
What change is being made?
Implement suppression of animation when running in Continuous Integration (CI) environments.
Why are these changes being made?
These changes resolve issue #702 by adding logic to check the CI environment variable and disable animations accordingly. This modification improves usability by preventing unnecessary animations in non-interactive environments like CI, which does not benefit from visual feedback. The changes keep interactive session output clean and focused on essential information.