type-fest icon indicating copy to clipboard operation
type-fest copied to clipboard

Published 20 hours ago • verified publisher icon sindresorhus

Add `ArraySlice` & `StringSlice` type #734

Open Max10240 opened this issue 1 year ago • 12 comments

array slice & string slice Issue

Max10240 avatar Nov 14 '23 06:11 Max10240

Thanks contribution. is it ready for review?

Emiyaaaaa avatar Nov 14 '23 06:11 Emiyaaaaa

@Emiyaaaaa Not quite yet😬, there are still some unit-tests and todo-list that require further work

Max10240 avatar Nov 14 '23 12:11 Max10240

@Emiyaaaaa Sorry I have been busy recently, now it is ready for review!

Max10240 avatar Nov 26 '23 14:11 Max10240

The formatting in the code examples needs some work. Make sure it follows the existing code style.

sindresorhus avatar Dec 02 '23 11:12 sindresorhus

Because of work, I may not have any free time until Saturday. I would greatly appreciate if someone is available to make corrections as per the comments!

Max10240 avatar Dec 03 '23 16:12 Max10240

@Max10240 There is no rush :)

It's your pull request, so it's your responsibility to address the comments.

sindresorhus avatar Dec 04 '23 13:12 sindresorhus

Of course, I just don't want it to drag on...

Max10240 avatar Dec 05 '23 17:12 Max10240

I've noticed some new types in internal.d.ts return the boolean when the input generic isn't expected, I think we could only care about expected input like all the other types in type-fast (there are lot of case in internal.d.ts). So I suggest return true/false if generic is computable and return never if we can't know the result. @Max10240

Emiyaaaaa avatar Dec 18 '23 04:12 Emiyaaaaa

@Emiyaaaaa Ignore the comment I quoted，I seem to have misunderstood something 😅; If the existing code style prefers "return never", I will make changes as you say! But, again, it will have to wait until Saturday...

Max10240 avatar Dec 19 '23 03:12 Max10240

@Emiyaaaaa please review it again :)

Max10240 avatar Dec 23 '23 05:12 Max10240

@Emiyaaaaa renamed files and so on❤️

Max10240 avatar Jan 06 '24 16:01 Max10240

Good progress on this, but it still requires a large amount of clean up and improvements when it comes to the doc comments and readme.

sindresorhus avatar Jan 08 '24 07:01 sindresorhus

Bump :)

sindresorhus avatar Feb 22 '24 06:02 sindresorhus

Any complaint if some one else picks this up and drags it over the line?

acron0 avatar Feb 27 '24 18:02 acron0

Sorry, I almost forgot the work of this thread, if there is no accident, I will update it today or tomorrow according to the modification suggestions

Max10240 avatar Feb 28 '24 01:02 Max10240

Any complaint if some one else picks this up and drags it over the line?

Of course not, I would appreciate it very much!

Max10240 avatar Feb 29 '24 16:02 Max10240

Tried my best to fix these comments，almost... please review it again, many thanks! @sindresorhus

Max10240 avatar Feb 29 '24 16:02 Max10240

@sindresorhus @Emiyaaaaa Is there any problem? ❤

Max10240 avatar Mar 06 '24 15:03 Max10240

Looks pretty good now.

sindresorhus avatar Mar 07 '24 09:03 sindresorhus

I noticed that two Add types are exported, but they don't mean the same thing. Maybe it would be better to rename the Add that means A + B to Sum? @sindresorhus @Max10240

Emiyaaaaa avatar Mar 08 '24 02:03 Emiyaaaaa

Yeah, it should be Sum.

sindresorhus avatar Mar 08 '24 04:03 sindresorhus

I noticed that two Add types are exported, but they don't mean the same thing. Maybe it would be better to rename the Add that means A + B to Sum? @sindresorhus @Max10240

sure

Max10240 avatar Mar 08 '24 07:03 Max10240

Awesome! Thanks for contributing this, @Max10240 🙏

sindresorhus avatar Mar 08 '24 15:03 sindresorhus