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

[update.pre.commit.hook] Updated pre-commit git hook #48

Merged
merged 2 commits into from
Feb 13, 2024

Conversation

scopplestone
Copy link
Collaborator

Do not allow executables to be committed.

@scopplestone scopplestone requested a review from kopperp August 24, 2023 13:38
@scopplestone scopplestone self-assigned this Aug 24, 2023
@kopperp
Copy link
Collaborator

kopperp commented Aug 29, 2023

Not an issue introduced in this MR but I wonder if we should try to find the paths to the binary in case somebody has already aliased it. Something along the lines of

DU_EXE=$(which "du")
LINE=$($DU_EXE -h "$file")

@kopperp kopperp force-pushed the update.pre.commit.hook branch from 01aceb4 to e2ead78 Compare September 13, 2023 20:13
@kopperp
Copy link
Collaborator

kopperp commented Oct 16, 2023

@scopplestone Are we good to merge it with those changes?

@kopperp kopperp force-pushed the update.pre.commit.hook branch from e2ead78 to e0012ab Compare February 12, 2024 13:21
Some commands might be already aliased to something else and not do what is expected
@kopperp kopperp force-pushed the update.pre.commit.hook branch from e0012ab to a41a538 Compare February 12, 2024 13:23
@scopplestone scopplestone merged commit d6156b8 into master Feb 13, 2024
2 checks passed
@scopplestone scopplestone deleted the update.pre.commit.hook branch February 13, 2024 11:08
@scopplestone scopplestone changed the title [update.pre.commit.hook]update.pre.commit.hook Updated pre-commit git hook [update.pre.commit.hook] Updated pre-commit git hook Feb 13, 2024
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