Skip to content
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

feat: auto-enable pre-commit checks when config exists #430

Merged
merged 1 commit into from
Jan 30, 2025

Conversation

ErikBjare
Copy link
Owner

@ErikBjare ErikBjare commented Jan 30, 2025

Pre-commit checks will now be enabled automatically when a .pre-commit-config.yaml file exists in any parent directory, in addition to when GPTME_CHECK=true is set.

This makes it easier to use pre-commit checks in projects that already have them configured, without requiring explicit opt-in.


Important

Automatically enable pre-commit checks in context.py if .pre-commit-config.yaml exists in any parent directory or GPTME_CHECK=true is set.

  • Behavior:
    • use_checks() in context.py now enables pre-commit checks if .pre-commit-config.yaml exists in any parent directory or if GPTME_CHECK=true is set.
    • run_precommit_checks() in context.py updated to reflect new enabling conditions.
  • Logging:
    • Adds warning in use_checks() if GPTME_CHECK=true is set but no .pre-commit-config.yaml is found.

This description was created by Ellipsis for 6b8faee. It will automatically update as commits are pushed.

Pre-commit checks will now be enabled automatically when a .pre-commit-config.yaml
file exists in any parent directory, in addition to when GPTME_CHECK=true is set.

This makes it easier to use pre-commit checks in projects that already have them
configured, without requiring explicit opt-in.
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 6b8faee in 21 seconds

More details
  • Looked at 65 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. gptme/util/context.py:302
  • Draft comment:
    Using shell=True can be a security risk. Consider avoiding it if possible, especially if the command string is constructed from external input.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The use of shell=True in subprocess.run can be a security risk if the command string is constructed from external input. In this context, it seems to be a static command, but it's still a good practice to avoid shell=True when not necessary.

Workflow ID: wflow_dM23SpfP4My29NkC


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 67.47%. Comparing base (e13af3f) to head (6b8faee).

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
gptme/util/context.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #430      +/-   ##
==========================================
+ Coverage   67.43%   67.47%   +0.03%     
==========================================
  Files          70       70              
  Lines        6130     6131       +1     
==========================================
+ Hits         4134     4137       +3     
+ Misses       1996     1994       -2     
Flag Coverage Δ
anthropic/claude-3-haiku-20240307 66.20% <85.71%> (+0.03%) ⬆️
openai/gpt-4o-mini 65.63% <85.71%> (+0.03%) ⬆️

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.

@ErikBjare ErikBjare merged commit 5cd9b1e into master Jan 30, 2025
7 checks passed
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