Closed bufdev closed 1 year ago
I propose following a similar paradigm for
connect-web
. wdyt @timostamm?
The license-header binary can update .proto files, and it can update generated code. We do both in connect-web and protobuf-es. eslint cannot do that.
I welcome a proposal that takes care of license headers with idiomatic tools, but it needs to include a way forward for the use cases that would cease to work.
The command-line tool license-header
happens to be written in Go, just like other tools happen to be written in Rust, C++, etc. Command-line tools have no specific language that they should be written in. There is no need to use a Makefile to use license-header
.
The license-header tool has recently been ported to JS: https://github.com/bufbuild/tools/tree/main/npm-packages/license-header
I'm with @fyockm on this one, we've already got the eslint toolchain enforcing formatting, might as well let it enforce licensing as well. I noticed the old PR (#16) required changing format of the license so I suggest an alternative PR (https://github.com/connectrpc/connect-query-es/pull/124) that does not require changes to the license format and still uses eslint as a standard to support fixing and validation.
@timostamm what do you think?
As I said earlier:
The license-header binary can update .proto files, and it can update generated code. We do both in connect-web and protobuf-es. eslint cannot do that.
I would much prefer we had a single way of enforcing license headers that works everywhere. We are actually not using the eslint toolchain to enforce formatting (see prettier), and the npm package addresses all of Drews concerns. It's already used here: https://github.com/bufbuild/jest-environment-jsdom
Oh man, i totally missed that comment. Yeah, generated code makes sense. Will adjust.
@paul-sachs, https://github.com/bufbuild/tools/pull/6 would give you the option to run license-header in the lint:code
script:
@timostamm yeah, I'm just trying to figure out how best to customize the ignore paths. Passing in individual ignores for each package is a little clunky. I might try to implement something in license-header that obeys the local .gitignore file. But for now, i'll work around it.
Ah, the tool already does that. Awesome!
In my opinion, using the Go binary does not make sense in an idiomatic JS repo for a few reasons:
One option is to use https://www.npmjs.com/package/eslint-plugin-notice to keep the headers up to date for us, like this https://github.com/bufbuild/connect-query/pull/16
I propose following a similar paradigm for
connect-web
. wdyt @timostamm?