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

Feature request: Support custom licenses #122

Closed
aripalo opened this issue Sep 27, 2020 · 11 comments · Fixed by #282
Closed

Feature request: Support custom licenses #122

aripalo opened this issue Sep 27, 2020 · 11 comments · Fixed by #282
Labels
good first issue Good for newcomers

Comments

@aripalo
Copy link

aripalo commented Sep 27, 2020

Problem

It seems that currently Projen supports following licenses:

  • Apache-2.0
  • MIT
  • MPL-2.0

… but for internally distributed (within a company) constructs / projects one should be able to set the license to something else (proprietary license).

Proposed solution

One idea could be allowing to specify the license as string:

project.license = `COWBOY LICENSE
This is a custom proprietary license. 
Only people wearing cowboy hats can use this project. 
Beware of our scary legal team.`;

Proposed Workaround

For a workaround one could use in .projenrc.js:

project.gitignore.exclude('LICENSE');

… but that doesn't work since Projen applies !/LICENSE after that:
Screenshot 2020-09-27 at 20 35 44

So at least having some way of "forcing" manual project.gitignore.exclude operations to be appended at the end would solve the issue.

@Chriscbr
Copy link
Contributor

Chriscbr commented Oct 7, 2020

Maybe there could also be an option to support no license, and have it automatically set private: "true" in the package.json to avoid unintentionally publishing a package. (Or this flag could be provided by a separate option)

@eladb
Copy link
Contributor

eladb commented Oct 7, 2020

Sounds good to me. Contributions are welcome

@pgarbe
Copy link
Contributor

pgarbe commented Nov 3, 2020

Instead of copying all the license, can't we just download them from https://github.com/github/choosealicense.com ?

@pgollucci
Copy link
Contributor

Is anything else download by projen yet ? Can we ,yes? @eladb thoughts?

@eladb
Copy link
Contributor

eladb commented Nov 3, 2020

Technically, synthesis includes yarn/npm install, which implies we are downloading stuff, so I am quite comfortable with this...

@jmourelos
Copy link
Contributor

Hi! I just decided to give it a try with a very simple solution: a flag "licensed".

  • If the flag is true (default value) the behavior is the current one.
  • If the flag is false:
    • No LICENSE file is generated
    • No entry regarding LICENSE is added to .gitignore
    • package.json license is set to "UNLICENSED"

Any thoughts?

@Chriscbr
Copy link
Contributor

Chriscbr commented Nov 7, 2020

Could we make the features you listed in the "false" section to automatically happen if license: 'UNLICENSED' is provided, to eliminate the extra flag? I feel like this would be a logical default behavior as long as it's documented in the docstring

@jmourelos
Copy link
Contributor

Currently, the argument "license" receives an SPDX identifier. There is already an SPDX identifier called "UNLICENSE" and I think it could lead to mistakes. An alternative would be to use something like "NO_LICENSE" (I only set package.json to UNLICENSED because otherwise, npm complains about the string not being an SPDX identifier, i.e., it is a special value for npm). Another option would be to use a union type.

Actually, I imagine that to accommodate new improvements regarding licenses it would make sense to have 2 fields in the future:

  • A flag to enable/disable adding a license (what I am doing with this PR). I am not quite sure if "licensed" is good naming, better would be just "license", but I just wanted to have a path to deprecate "license" and tell users they should use licenseOptions to set the SPDX ID in the future.
  • A licenseOptions field that, based on some ideas from this issue, I imagine could have fields like:
    • URL to download the license from
    • Tokens to replace with author name, year, project url, etc. in the downloaded license (e.g. [name of copyright owner] is used for the licenses stored in the repo).
    • spdxId

If we would like to add the possibility of downloading the licenses I think it would make sense to keep the flag.

@jmourelos
Copy link
Contributor

Just rereading the thread I think probably @pgarbe just meant specifying the SPDX identifier as we do currently and use by default only choosealicense.com to download the licenses. That would not require the user to provide the URL since it could be generated programmatically and then licenseOptions would not be needed.

@Chriscbr
Copy link
Contributor

Chriscbr commented Nov 9, 2020

Ah that makes sense - thanks for the helpful explanation!

Yeah I guess I wouldn't mind using 'NO_LICENSE' to avoid confusion, but it's fair that a flag works just as well. Or putting all related fields in a licenseOptions like you suggested. I don't really have a strong opinion here.

Tokens to replace with author name, year, project url, etc. in the downloaded license (e.g. [name of copyright owner] is used for the licenses stored in the repo).

Some of these placeholder substitutions are currently handled using other fields:

projen/src/license.ts

Lines 35 to 38 in e4f5d43

this.text = fs.readFileSync(textFile, 'utf-8')
.replace('[yyyy]', years)
.replace('[name of copyright owner]', owner);
}

Any other missing placeholders substitutions should definitely be added! But whenever possible, these fields should be passed from the rest of the project options -- not provided via license-specific options. This way the user only has to specify properties at the project level (such as the author name, year, etc.), and the fields will automatically propagate to all appropriate files, including the license.

That said it's also helpful to have escape hatches for specifying things like custom license texts, for example.

@jmourelos
Copy link
Contributor

Thanks a lot for the feedback!

Yeah I guess I wouldn't mind using 'NO_LICENSE' to avoid confusion, but it's fair that a flag works just as well. Or putting all related fields in a licenseOptions like you suggested. I don't really have a strong opinion here.

I don't have a strong opinion either, so I will just let it stay as it is currently. I set the PR to ready for review.

Any other missing placeholders substitutions should definitely be added! But whenever possible, these fields should be passed from the rest of the project options -- not provided via license-specific options. This way the user only has to specify properties at the project level (such as the author name, year, etc.), and the fields will automatically propagate to all appropriate files, including the license.

Definitely, I was thinking of cases where the information would be specific for the license, but I can see that that is not very likely.

@mergify mergify bot closed this as completed in #282 Nov 13, 2020
mergify bot pushed a commit that referenced this issue Nov 13, 2020
* Small fix to util test function directorySnapshot: return was preventing
the function to transverse the whole tree.
* Added filtering to util test function synthSnapshot to limit size of the
snapshot

Fixes #122
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants