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

chore: remove licensee as a devDependency #7380

Merged
merged 1 commit into from
Apr 24, 2024
Merged

Conversation

lukekarrys
Copy link
Contributor

@lukekarrys lukekarrys commented Apr 16, 2024

It is only used during CI and we can call it with npx

@lukekarrys lukekarrys requested a review from a team as a code owner April 16, 2024 18:04
@ljharb
Copy link
Contributor

ljharb commented Apr 16, 2024

I would strongly suggest keeping licensee; especially with its community corrections overrides, it's going to be more robust than a homegrown script.

@wraithgar wraithgar changed the title chore: remove licensee in favor of own script chore: remove licensee as a devDependency Apr 24, 2024
@ljharb
Copy link
Contributor

ljharb commented Apr 24, 2024

This is much better, but will still break whenever v11 comes out - what's the benefit of hiding the dependency information by not listing it explicitly? Listing it explicitly means tooling is aware of it.

@wraithgar
Copy link
Member

It uses a lot of the same subdependencies as npm, but it uses older versions of them. There is quite a bit of cognitive load trying to manage them in the tree and disregard them as not something to update.

@ljharb
Copy link
Contributor

ljharb commented Apr 24, 2024

when you say "manage them in the tree", do you mean in the lockfile? or do you mean that npm vendors dev deps as well as runtime deps? (perhaps only runtime deps should be vendored, if so)

@wraithgar
Copy link
Member

We update deps in npm every release, it is a very mentally intensive process because of the nature of npm, every update is done individually and checked. Checking for outdated deps is also done every time and these only make the process more exhausting. This is a quality of life improvement for the folks who maintain npm. Arguments coming from the perspective of ideals or the purity of architecture are not going to carry any weight here.

@lukekarrys
Copy link
Contributor Author

lukekarrys commented Apr 24, 2024

If npx licensee breaks when v11 comes out, we will either fix it or use npx licensee@10 with a todo to migrate in the future. I think its actually good we use npx licensee here so we know when v11 comes out. Keeping it as a dev dep would give us the same benefit of course.

But currently licensee creates a lot of noise when running npm ls locally. Reducing this noise is worth whatever time we spend in the future fixing licensee or staying on an outdated version.

@ljharb
Copy link
Contributor

ljharb commented Apr 24, 2024

Alrighty. Seems like a motivation to change the workflow so npm maintainers' quality of life isn't dramatically worse than everyone else's :-) but obv its up to yall

@lukekarrys
Copy link
Contributor Author

The best future here would be npm outdated (or any command) taking any --query as a parameter. We'd codify that via a run-script in our repo and then npm run my-outdated would skip anything we consider non-essential to our usual outdated checking. The dev vs prod binary doesn't cut it here.

But that will take substantially longer than replacing licensee with npx licensee. 😄

It is only used during CI and we can call it with npx
@wraithgar wraithgar merged commit ea66e95 into latest Apr 24, 2024
18 checks passed
@wraithgar wraithgar deleted the lk/license-script branch April 24, 2024 17:17
@github-actions github-actions bot mentioned this pull request Apr 24, 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.

3 participants