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

Add Nano Colors mention as inspiration #78

Closed
ai opened this issue Sep 26, 2021 · 5 comments
Closed

Add Nano Colors mention as inspiration #78

ai opened this issue Sep 26, 2021 · 5 comments

Comments

@ai
Copy link
Contributor

ai commented Sep 26, 2021

Recent !s optimization was clearly backport from Nano Colors. Can you add a note that Nano Colors was an inspiration for the project.

@ai ai changed the title Add mention Nano Colors as inspiration Add Nano Colors mention as inspiration Sep 26, 2021
@ai
Copy link
Contributor Author

ai commented Sep 26, 2021

babel/babel#13783 (comment)

You edge case suggestions was very useful, but you didn’t mention !s fix there. You didn’t create an initial boost, and you didn’t fix it to name yourself as author.

I most of the cases, I am OK with idea copying between projects. But in this case, I think I have the right to do another way and ask to mention me if you copied this performance boost. You may mention me in a way “Initially create in Nano Colors and then fixed by team work of me and Andrey Sitnik”.

@jorgebucaran
Copy link
Owner

I'm afraid you can't claim !s as your own invention. It's a pretty obvious fix by looking at the stack trace.

Screen Shot 2021-09-26 at 18 07 14

I'm cool with borrowing ideas from each other too, of course.

@ai
Copy link
Contributor Author

ai commented Sep 26, 2021

Anyway, it was not did by you.

And it was a fix for initial my idea that === '' compare is slow. Without that changes, this discussion would not be started.

You do not need this fix in Colorette for undefined argument. You added !s check only because you saw that it was very effective in Nano Colors.

@jorgebucaran
Copy link
Owner

jorgebucaran commented Sep 26, 2021

Sorry, but I must disagree on behalf of the little programmer out there. You just can't take credit for a !s.

!s was the smallest change I had to make to avoid the explicit s === undefined, so I went with that. I could've reversed the expression just as well and not use !s at all:

!(!s && (s === "" || s === undefined))
s || (s !== "" && s !== undefined)
s || !(s === "" || s === undefined)

They're all roughly equivalent and produce the same result. I'll tell you what, I'll switch to another one and close here.

@ai
Copy link
Contributor Author

ai commented Sep 26, 2021

You lost the whole idea of optimization.

You saw from Nano Colors that s !== "" && s !== undefined is slow. You copied the fix of this slow part.

Now you are changing !x to x instead of do an honor thing and mention an author of original work.

Repository owner locked as too heated and limited conversation to collaborators Sep 26, 2021
@jorgebucaran jorgebucaran pinned this issue Sep 26, 2021
@jorgebucaran jorgebucaran unpinned this issue Sep 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants