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.

Sign up for GitHub

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

Jump to bottom

Add make targets for common checks + fixes #661

Merged
merged 1 commit into from
Feb 26, 2024
Merged

Add make targets for common checks + fixes #661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
fmtcheck:
Copy link
Member

@radditude radditude Feb 24, 2024 •
edited

Choose a reason for hiding this comment

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

I can't help but notice that this file is itself missing a copyright header 😆 I don't know if that matters - the copywrite tool doesn't seem unhappy about it so maybe there's a global allowlist.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch! I don't think it matters at this point.

The reason is that the tool basically does not know how to add the header to makefiles, it's not that it would be intentionally ignored.

See https://github.com/hashicorp/copywrite/blob/0f077594eee1ea09d721a7e6fc9f07302aaff7fe/addlicense/main.go#L360-L392

Also the same issue applies to the upstream project our copywrite has forked, although it does have some pending PRs:

To be fair there is support for *.cmake but I don't have enough context/knowledge to tell what that is and what's the relationship with Makefile or GNUMakefile and I am not quite convinced there's a value in learning that just to add the license headers. I would perceive the licensing risk with makefiles in general to be relatively low in comparison with the rest of the codebase.

Feel free to correct me if I'm wrong in any of the above though!

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, it seems pretty harmless to leave the Makefile un-headered!

"$(CURDIR)/scripts/gofmtcheck.sh"

fmtfix:
gofmt -w ./

vetcheck:
go vet ./...

copyrightcheck:
go run github.com/hashicorp/copywrite@latest headers --plan

copyrightfix:
go run github.com/hashicorp/copywrite@latest headers

check: copyrightcheck vetcheck fmtcheck

fix: copyrightfix fmtfix
5 changes: 5 additions & 0 deletions scripts/gofmtcheck.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/usr/bin/env bash
# Copyright (c) HashiCorp, Inc.
# SPDX-License-Identifier: MPL-2.0

if [[ -n $(gofmt -l ./) ]]; then echo "Please run gofmt -w ./ to format code"; exit 1; fi;