-
-
Notifications
You must be signed in to change notification settings - Fork 40
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(dev): add hooks setup and pre-push hook with black formatter #101
base: master
Are you sure you want to change the base?
Conversation
a39cabc
to
7ba628d
Compare
This is a nice suggestion. Though introducing an alternate CLI does not quite align with the rest of the project. Additionally, registering "install-hooks" globally could potentially annoy some users. To avoid this, it might be better to integrate the "install-hooks" command within ClaudeSync's CLI under "hooks". This commit also need signing. |
4feb24c
to
bdb6d65
Compare
bdb6d65
to
0295a74
Compare
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.
Upon further contemplation though this is nice to have for python developers i.e. someone working on claudesync. It’s difficult to see how it benefits the wider user base. It doesn’t feel like an entirely natural thing to have in claudesync. In java we would typically have a build tool plugin that would install the git hook, we wouldn’t distribute it with the software itself.
@@ -0,0 +1,77 @@ | |||
#!/usr/bin/env python3 |
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.
This approach didn’t work on Windows because only the python
alias is mapped, leading to the error:
/usr/bin/env: 'python3': No such file or directory
.
Since this issue could affect a number of users, it would be more robust to use #!/usr/bin/env python
in the shebang for better cross-platform compatibility. To ensure the script runs with Python 3, you can add a runtime version check like this:
import sys
if sys.version_info.major < 3:
print("Error: This script requires Python 3. Please install and configure it.")
sys.exit(1)
This ensures that the script will fail gracefully if the system default python
points to Python 2. It also avoids requiring users to set up the python3
alias, which may not be available by default on Windows.
This pull request has been marked as stale due to 7 days of inactivity. Please remove the stale label or comment to keep it open, otherwise, it will be closed in 3 days. |
I understand your point about this being primarily a development tool rather than a user-facing feature. Would you be open to keeping this functionality but moving it to a separate development tools folder (e.g., in a {root}/tools ) with its own CLI command claudesync-dev (or maybe just to be run as an executable script)? This would keep it available for claudesync developers while not including it in the main package distribution. Let me know if you'd like me to restructure the PR this way or if you'd prefer not to include this feature at all. |
Add Git Hooks Support with Black Code Formatting
Overview
This PR adds support for Git hooks in ClaudeSync, starting with a pre-commit hook that automatically formats Python code using black before each commit.
Features
claudesync hooks install
commandImplementation Details
hooks.py
module to handle hook installation and managementpre_commit.py
template that formats staged Python files using blackpyproject.toml
to include black as optional dependencyUsage
After installing ClaudeSync, users can install the hooks with:
Comments
@jahwag I dont really know if this is a feature you want to incorporate to the project, I know some devs are not friends of this hooks workflow. Feel free to just decline this PR.