Skip to content

Add pre-commit hook for running addlicense#1095

Closed
rickeylev wants to merge 3 commits into
bazel-contrib:mainfrom
rickeylev:add-license-precommit
Closed

Add pre-commit hook for running addlicense#1095
rickeylev wants to merge 3 commits into
bazel-contrib:mainfrom
rickeylev:add-license-precommit

Conversation

@rickeylev

Copy link
Copy Markdown
Collaborator

Integrate the addlicense.sh script with the pre-commit config so that it automatically runs when git committing.

@alexeagle

Copy link
Copy Markdown
Contributor

The standard way to do this is expose a pre-commit hook from the addlicense repo, so that everyone can add it, and you don't need a shell script or a new .pre-commit-hooks.yaml here

@rickeylev

Copy link
Copy Markdown
Collaborator Author

Thanks for the pointer. I sent google/addlicense#143. Once accepted (I don't see why it wouldn't be...), we'll have to wait for them to do a release with a stable tag to use in the configs. I have another PR ready for when that happens.

In the meantime, I found the docs that tell how to correctly setup a local hook and updated the code to do that. PTAL.

@f0rmiga

f0rmiga commented Feb 25, 2023

Copy link
Copy Markdown
Member

It bothers me that pre-commit doesn't use Bazel for the binaries. The best way to deal with this is:

  • Pull the binaries using http_archives.
  • Create an alias with selects to resolve the platform.
  • Create a shell wrapper that cd into BUILD_WORKSPACE_DIRECTORY and execute the binary:
#!/usr/bin/env bash

set -o errexit -o nounset

cd "${BUILD_WORKSPACE_DIRECTORY}"

exec "${ADDLICENSE_BINARY}" "$@"
  • Add the entry attribute to the pre-commit hook with entry: "bazel run //<pre-commit wrappers package>:addlicense --".

@alexeagle

Copy link
Copy Markdown
Contributor

Pre-commit running Bazel is not great due to pre-commit/pre-commit#1003 (comment)
I think it does a pretty good job of providing the hermetic tooling and there's no need for Bazel where there's no dependency graph IMO

@chrislovecnm

Copy link
Copy Markdown
Contributor

Can we get this into a test? CI should fail on this.

@chrislovecnm

Copy link
Copy Markdown
Contributor

@rickeylev ping

@github-actions

Copy link
Copy Markdown
Contributor

This Pull Request has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days.
Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_python!

@github-actions github-actions Bot added the Can Close? Will close in 30 days if there is no new activity label Dec 10, 2023
@github-actions

Copy link
Copy Markdown
Contributor

This PR was automatically closed because it went 30 days without a reply since it was labeled "Can Close?"

@github-actions github-actions Bot closed this Jan 10, 2024
@rickeylev rickeylev deleted the add-license-precommit branch June 29, 2026 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Can Close? Will close in 30 days if there is no new activity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants