Skip to content

Conversation

@zhouzaihang
Copy link

Ensure makefiles also get license headers.

Copy link
Contributor

@flwyd flwyd left a comment

Choose a reason for hiding this comment

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

Please sync your branch with master, too.

// handle various cmake files
if base == "cmakelists.txt" || strings.HasSuffix(base, ".cmake.in") || strings.HasSuffix(base, ".cmake") {
lic, err = executeTemplate(tmpl, data, "", "# ", "")
} else if base == "Makefile" || base == "makefile" || strings.HasSuffix(base, ".mk") { // handle various make files
Copy link
Contributor

Choose a reason for hiding this comment

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

cmake needs special support because it needs to look at multiple parts of the filename. I think you can just add this to the case statement above, similar to Gemfile.


# set env
RUN go env -w GO111MODULE=on
RUN go env -w GOPROXY=https://goproxy.cn,direct
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert the changes to this file.

n = len(b)
}
return bytes.Contains(bytes.ToLower(b[:n]), []byte("copyright")) ||
return bytes.Contains(bytes.ToLower(b[:n]), []byte("# copyright")) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

This change doesn't seem necessary: "# copyright" also contains the string "copyright".

},
{
[]string{"cmakelists.txt", "f.cmake", "f.cmake.in"},
[]string{"cmakelists.txt", "f.cmake", "f.cmake.in", "makefile", "Makefile", "f.mk"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Create a separate test group for makefiles; they're different than cmake.

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