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

[bug] dependency cli-color has licensing issues #56

Closed
DerekNonGeneric opened this issue Jul 17, 2022 · 4 comments · Fixed by #117
Closed

[bug] dependency cli-color has licensing issues #56

DerekNonGeneric opened this issue Jul 17, 2022 · 4 comments · Fixed by #117

Comments

@DerekNonGeneric
Copy link
Member

According to Socket, cli-color has issues related to licensing. We are currently using the MIT license for the packages we publish on npm and intend to continue doing so. This is kind of a hard requirement since a lot of projects have a strict requirement of having an MIT license. It is also important that the dependencies of our dependencies are MIT licensed as well.

We may be able to swap this one out for colorette, which is MIT-licensed (silver quality) and maintained by my good friend @jorgebucaran. It has no dependencies.

Refs: DerekNonGeneric/loader339#11 (comment)

@ghost
Copy link

ghost commented Oct 14, 2022

Hi @DerekNonGeneric,

I posted a PR for this ussue, but I'm having some problems with some of the tests required by your repo:
I managed to fix all but two, both of them are complaining against the autogenerated index.js file (npm run prepare)

I'd think that this file, being automatically generated, should be excluded from these tests, but if you'd like, I'd be more than happy to apply some changes to it

@DerekNonGeneric
Copy link
Member Author

Hi @dadolhay, thanks; that would be cool by me.

If you prefer, you can also exclude the out-generated file from your changes.

@ghost
Copy link

ghost commented Oct 14, 2022

@DerekNonGeneric

I made two changes:

  • I started using normal named imports (renaming them one by one to avoid naming collision)
  • I completely excluded index.js from the PR

With this it seems all tests are passing, but please note, that the functionality of index.js and index.ts differs

@DerekNonGeneric
Copy link
Member Author

Yeah, thanks! Noted; that would need to be taken care of in a separate PR. The issue for it is #115. :)

DerekNonGeneric added a commit that referenced this issue Oct 14, 2022
Fixes: #56

Signed-off-by: Denes Dolhay <[email protected]>
Signed-off-by: Derek Lewis <[email protected]>

Co-authored-by: Denes Dolhay <[email protected]>
Co-authored-by: Derek Lewis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant