Skip to content

refactor: replace colors with colorette#803

Closed
jorgebucaran wants to merge 32 commits into
nodeSolidServer:release/v5.0.0from
jorgebucaran:master
Closed

refactor: replace colors with colorette#803
jorgebucaran wants to merge 32 commits into
nodeSolidServer:release/v5.0.0from
jorgebucaran:master

Conversation

@jorgebucaran
Copy link
Copy Markdown
Contributor

@jorgebucaran jorgebucaran commented Sep 30, 2018

👋 Hello maintainers.

Summary

This PR replaces colors with colorette. Disclaimer: I'm the author of colorette. This is a refactor change and does not intend to introduce breaking changes. Please let me know if this looks good or not and if you'd like me to make any changes.

Colorette is both lighter and faster than the colors packages. It's also "safe" by default, it doesn't mutate the string prototype. Terminal color support is autodetected, so our console output will not contain unreadable symbols if the target terminal does not support color.

Tests and coverage are at 100%.

Size Comparison

Package Size
colors install size
colorette install size

Runtime Performance

All tests run on a 2.4GHz Intel Core i7 CPU with 16 GB memory. Complete benchmark results here and implementation here.

# Using Styles
colors × 64,999 ops/sec
> colorette × 709,733 ops/sec

# Combining Styles
colors × 246,004 ops/sec
> colorette × 2,049,555 ops/sec

# Nesting Styles
colors × 144,403 ops/sec
> colorette × 389,825 ops/sec

RubenVerborgh and others added 23 commits September 14, 2018 16:33
@RubenVerborgh
Ensure that tests use test config.
9a83e3b
@RubenVerborgh
Remove SonarJS.
e2ad77d 
Doesn't work with symlinks,
and excluding those symlinks doen't work either.
https://github.com/SonarSource/SonarJS/issues/1057
@RubenVerborgh
Merge pull request #776 from solid/fix/tests-config-path
dcbf792 
Ensure that tests use test config
@RubenVerborgh
Ensure all dependencies use oidc-rp 0.8.0.
9c741b2
@RubenVerborgh
Merge pull request #791 from solid/feature/oidc-rp-0.8.0
f74b287 
Ensure all dependencies use oidc-rp 0.8.0
@RubenVerborgh
Move routeResolvedFile to utils.
f375d3b
@RubenVerborgh
Bundle Mashlib.
91ae4e1 
Closes #773.
@RubenVerborgh
Allow session to reach auth handlers.
4d0c094 
Otherwise, third-party origins cannot log the user out.
@RubenVerborgh
Merge pull request #792 from solid/feature/bundle-mashlib
c1cf810 
Use local Mashlib
@RubenVerborgh
Merge pull request #793 from solid/fix/allow-external-logout
d256943 
Do not block third-party cookies from reaching auth handlers
@RubenVerborgh
Release version 4.1.4 of the npm package.
7e7fb8a
@RubenVerborgh
Use oidc-op-0.4.0.
3bf3e7c
@RubenVerborgh
Test global package installation on Travis.
1ee8d2f
@RubenVerborgh
Check for GitHub dependencies on Travis.
8b934f7
@mohamedmansour
Improve the error messaging for permissions
4bcaf21 
If an invalid setup with directory permissions happened, it doesn't mean an invalid port. It usually implies it is a setup error. Add the handling for the mkdir sys call.
@mohamedmansour
Fix standard nits
9e14768
@RubenVerborgh
Send errors as plain text.
3071b12
@RubenVerborgh
Extend list of reserved IP addresses.
a3fdde2 
Taken from jtdowney/private_address_check#3
@RubenVerborgh
Disable CORS proxy by default.
13a365a
@RubenVerborgh
Add mechanism for static .well-known files.
357a59f
@RubenVerborgh
Add security.txt.
1f6d690
@RubenVerborgh
Use default popup URI.
d5d1c38
@RubenVerborgh
Merge pull request #795 from solid/feature/oidc-op-0.4.0
272c1be 
Upgrade to oidc-op-0.4.0
@kjetilk
Copy link
Copy Markdown
Member

kjetilk commented Sep 30, 2018

Looks cool! Could you please make sure Travis passes by updating the locks?

@kjetilk kjetilk added this to the 5.0.0 milestone Sep 30, 2018
@jorgebucaran
Copy link
Copy Markdown
Contributor Author

jorgebucaran commented Sep 30, 2018

@kjetilk Thank you. Yes, done.

@kjetilk
Copy link
Copy Markdown
Member

kjetilk commented Sep 30, 2018

OK, good! And, BTW, we had some questions about whether we should have a CLA, and we don't know, but we'd just figured we'd ask if you're OK with the MIT license?

@kjetilk kjetilk changed the base branch from master to release/v5.0.0 September 30, 2018 19:06
RubenVerborgh
RubenVerborgh suggested changes Sep 30, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@RubenVerborgh RubenVerborgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs rebase.

@jorgebucaran
Copy link
Copy Markdown
Contributor Author

jorgebucaran commented Oct 1, 2018

@kjetilk And, BTW, we had some questions about whether we should have a CLA, and we don't know, but we'd just figured we'd ask if you're OK with the MIT license?

I'm perfectly OK with the MIT license.

@RubenVerborgh Sure. Rebase completed.

@RubenVerborgh
Copy link
Copy Markdown
Contributor

RubenVerborgh commented Oct 1, 2018

@jorgebucaran There still appear to be some conflicts though.

@jorgebucaran
Copy link
Copy Markdown
Contributor Author

jorgebucaran commented Oct 1, 2018 •
edited
Loading

@RubenVerborgh You are right. Sorry, I don't know how to fix the conflict (I don't understand why other commits were added to this PR either), so I've created a new identical PR here. #812

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@RubenVerborgh RubenVerborgh RubenVerborgh requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@kjetilk kjetilk

Labels

enhancement

Projects

None yet

Milestone

5.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@jorgebucaran @kjetilk @RubenVerborgh @mohamedmansour