Skip to content

Conversation

@wtrocki
Copy link
Member

@wtrocki wtrocki commented Mar 11, 2025

Proposed changes

Adding specific golang precommit hooks to help with streamlining PR creation.
Changes are only run when go file change is detected in changeset.

@wtrocki wtrocki requested a review from a team as a code owner March 11, 2025 09:53
@wtrocki
Copy link
Member Author

wtrocki commented Mar 11, 2025

FYI @andreaangiolillo @blva

@wtrocki wtrocki requested a review from Copilot March 11, 2025 09:59
Comment on lines 22 to 23
# Return to the original directory
cd -
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we use pushd popd here please

Copy link
Member Author

Choose a reason for hiding this comment

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

No problem! adding.


# Run unit tests for Go files
echo "Running Go unit tests..."
make unit-test
Copy link
Collaborator

@andreaangiolillo andreaangiolillo Mar 11, 2025

Choose a reason for hiding this comment

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

Since we have most of this in the cli makefile, can we have here

#!/usr/bin/env bash
pushd tools/cli
make pre-commit

so that we reuse the logic in the makefile in the cli

Copy link
Member Author

Choose a reason for hiding this comment

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

Awesome! I missed that precommit setup!

Copy link
Collaborator

@andreaangiolillo andreaangiolillo left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for all the changes

@wtrocki wtrocki merged commit 2a761db into main Mar 11, 2025
12 checks passed
@wtrocki wtrocki deleted the commit-hook-golang branch March 11, 2025 12:30
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.

3 participants