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

Set Up GolangCI Linter While Commit #155

Conversation

TheCoderAdi
Copy link

Description

This pull request introduces new Git hooks to improve the development workflow by ensuring code quality before commits. The most important changes include the addition of a pre-commit hook for running golangci-lint on staged Go files and a setup script for installing Git hooks.

Git Hooks Setup:

  • .github/hooks/pre-commit: Added a pre-commit hook script to run golangci-lint on staged Go files, ensuring linting is performed before committing.
  • setup-hooks.sh: Added a setup script to install Git hooks from the .github/hooks directory into the local repository's .git/hooks directory.

Fixes

Fixes #91

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

To verify that the Git hooks are installed and work as expected, follow these steps:

Reproduction Steps:

  1. Clone the repository (if not already cloned):

    git clone <repository-url>
    cd <repository-name>
  2. Run the setup script to install hooks:

    ./setup-hooks.sh
    • Expected Output:
      🔗 Setting up Git hooks...
      ✅ Hook 'pre-commit' installed.
      🎉 Git hooks setup complete!
      
  3. Make changes to a Go file and stage it:

    echo 'package main; func main() {}' > test.go
    git add test.go
  4. Attempt to commit the changes:

    git commit -m "Test pre-commit hook"
    • If there are linting errors: The commit should fail with a message showing linting issues.
    • If no issues are found: The commit should succeed.

Expected Behavior

  • If a Go file has linting issues, the commit should be blocked.
  • If all files pass linting, the commit should proceed successfully.
  • Running ./setup-hooks.sh should correctly install hooks in .git/hooks/.

@TheCoderAdi TheCoderAdi force-pushed the feat/add-golangci-linter-precommit branch from 1692473 to d2c0973 Compare March 14, 2025 20:21
@zriyanshdz
Copy link
Collaborator

@TheCoderAdi let us know if this PR is review ready, you can request a request from @hash-data when ready

@TheCoderAdi
Copy link
Author

@TheCoderAdi let us know if this PR is review ready, you can request a request from @hash-data when ready

Yes it is ready for review, where can I request him?
In slack?

@TheCoderAdi
Copy link
Author

Hello @hash-data ,

Can you please review my changes

@hash-data hash-data requested a review from vikash390 March 17, 2025 15:31
@hash-data
Copy link
Collaborator

@TheCoderAdi have you tested it, as the requirement was to fail commit if Golangci failes. I tested it and it is not fulfilling the requirement.

@TheCoderAdi
Copy link
Author

@TheCoderAdi have you tested it, as the requirement was to fail commit if Golangci failes. I tested it and it is not fulfilling the requirement.

Yes, I have tested it,Check this video

2025-03-18.16-56-03.mp4

@hash-data
Copy link
Collaborator

@TheCoderAdi have you tested it, as the requirement was to fail commit if Golangci failes. I tested it and it is not fulfilling the requirement.

Yes, I have tested it,Check this video

2025-03-18.16-56-03.mp4

Hi @TheCoderAdi thanks for sharing the video please check this as well : https://drive.google.com/file/d/1dzMyt0jHCPNjpmSktK7XbtHdQ3uuKkGr/view?usp=sharing

@TheCoderAdi
Copy link
Author

@TheCoderAdi have you tested it, as the requirement was to fail commit if Golangci failes. I tested it and it is not fulfilling the requirement.

Yes, I have tested it,Check this video
2025-03-18.16-56-03.mp4

Hi @TheCoderAdi thanks for sharing the video please check this as well : https://drive.google.com/file/d/1dzMyt0jHCPNjpmSktK7XbtHdQ3uuKkGr/view?usp=sharing

Thank you for sharing the video. I will review it and get back to you with my findings.

@TheCoderAdi
Copy link
Author

Hey @hash-data ,

I have fixed the code can you review it now

2025-03-18.19-24-03.mp4

@TheCoderAdi
Copy link
Author

Hello @hash-data ,

Any further modification from my side?

@hash-data
Copy link
Collaborator

Hello @hash-data ,

Any further modification from my side?

will test it.

@TheCoderAdi
Copy link
Author

Hello @hash-data ,
Any further modification from my side?

will test it.

Okay

Copy link
Contributor

Choose a reason for hiding this comment

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

is there a way we can avoid running this? basically it just comes pre-installed when we clone the repo.

Copy link
Author

Choose a reason for hiding this comment

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

We can add a .gitconfig file in the root of the project to set the core.hooksPath to .github/hooks.
However, to avoid permission issues and ensure the hooks are correctly picked up after cloning, we'll still need to run:
git config core.hooksPath .github/hooks
Do you want me to proceed with this approach?

Comment on lines +7 to +11
# Check if golangci-lint is installed
if ! command -v golangci-lint >/dev/null 2>&1; then
echo "❌ golangci-lint is not installed. Please install it first."
exit 1
fi
Copy link
Contributor

@shubham19may shubham19may Mar 22, 2025

Choose a reason for hiding this comment

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

Install it as a part of running setup-hooks.sh script if not installed

Copy link
Contributor

Choose a reason for hiding this comment

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

or simply run
make golangci && make gofmt

Copy link
Author

Choose a reason for hiding this comment

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

Okay So I will add this command make golangci && make gofmt in the setup file and will remove the checking in the pre-commit file

Copy link
Contributor

Choose a reason for hiding this comment

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

Whats the better approach, current one that you have implemented or https://goangle.medium.com/golang-improving-your-go-project-with-pre-commit-hooks-a265fad0e02f

do let me know after the research

Copy link
Author

@TheCoderAdi TheCoderAdi Mar 22, 2025

Choose a reason for hiding this comment

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

yes, I thinks this one is more cleaner and also does not have any permission error and we have to run
command pip install pre-commit and pre-commit install.

Should I go ahead and prepare a .pre-commit-config.yaml file?

@TheCoderAdi
Copy link
Author

Hello @shubham19may ,

Thank you for all your valuable inputs. I will review them and once completed, I will reach out to you for your feedback.

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.

Set Up GolangCI Linter While Commit
4 participants