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

refactor: add npm actions to generate the licenses files #2116

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

sbruens
Copy link
Contributor

@sbruens sbruens commented Aug 12, 2024

Ideally this is done at build time instead of manually, but I don't have time to look into that right now. Leaving that for a future PR.

@sbruens sbruens marked this pull request as ready for review August 12, 2024 18:55
@sbruens sbruens requested a review from a team as a code owner August 12, 2024 18:55
@sbruens sbruens requested a review from daniellacosse August 12, 2024 18:58
@sbruens
Copy link
Contributor Author

sbruens commented Aug 12, 2024

@daniellacosse I couldn't get it to work properly on workspaces without specifying both the root and workspace package.json, since some dependencies are installed at ./node_modules/ and others at ./workspaces/node_modules/. Open to suggestions.

@daniellacosse
Copy link
Contributor

@daniellacosse I couldn't get it to work properly on workspaces without specifying both the root and workspace package.json, since some dependencies are installed at ./node_modules/ and others at ./workspaces/node_modules/. Open to suggestions.

Perhaps you could have a root license file sent to output that gets appended to the result in each workspace?

Do we need to commit these? Can we just generate them at build time? If not, they should probably live in a LICENSE file at the root of each workspace (e.g. client/LICENSE)?

@fortuna fortuna removed the request for review from a team August 12, 2024 19:56
@sbruens
Copy link
Contributor Author

sbruens commented Aug 12, 2024

Do we need to commit these? Can we just generate them at build time?

That's what I suggested in the PR description. Agree it should be at build time but I don't have time for that right now. This at least brings the license files up-to-date.

If not, they should probably live in a LICENSE file at the root of each workspace (e.g. client/LICENSE)?

They shouldn't live in LICENSE, that's the repo's own license. This file is the file we distribute with binaries to list all dependency licenses.

@sbruens
Copy link
Contributor Author

sbruens commented Aug 12, 2024

@daniellacosse I couldn't get it to work properly on workspaces without specifying both the root and workspace package.json, since some dependencies are installed at ./node_modules/ and others at ./workspaces/node_modules/. Open to suggestions.

Perhaps you could have a root license file sent to output that gets appended to the result in each workspace?

That doesn't really solve the problem that there's no way to identify which root dependencies belong to which workspace. It's probably fine like this.

@sbruens sbruens force-pushed the sbruens/license-file branch from 0f46e10 to 40d6fa2 Compare September 4, 2024 17:02
@sbruens sbruens enabled auto-merge (squash) September 4, 2024 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants