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

fix: "no-dangle-underscore", and other nits. #621

Merged
merged 17 commits into from
Apr 22, 2024
Merged

Conversation

eranhirsch
Copy link
Collaborator

And follow up with going over new rules and old rules and making sure our config is top-notch.

@eranhirsch eranhirsch marked this pull request as ready for review April 8, 2024 13:36
@eranhirsch eranhirsch requested a review from cjquines April 8, 2024 13:37
@eranhirsch eranhirsch marked this pull request as draft April 8, 2024 13:42
@eranhirsch
Copy link
Collaborator Author

eranhirsch commented Apr 8, 2024

This will have to wait for typescript-eslint to also update to v9, in the meantime this PR can't build in CI.

Watching: typescript-eslint/typescript-eslint#8211

@eranhirsch eranhirsch added the v2 PRs that are part of the v2 release label Apr 8, 2024
eslint.config.js Outdated Show resolved Hide resolved
src/pipe.ts Outdated Show resolved Hide resolved
src/sample.ts Outdated
@@ -102,6 +102,7 @@ export function sample(...args: ReadonlyArray<unknown>): unknown {
return purry(sampleImplementation, args);
}

// eslint-disable-next-line max-statements -- TODO: Can we simplify or split this?
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: i see this function has a lot of guards, which we don't really have in other functions. for example, for chunk, we don't check that size is a positive integer. if we eliminate this, that'd bring us under the limit right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think you are right and it also looks like how the built-in functions chose to handle this: for example [1,2,3].slice(1.5).

I'll remove them

Copy link

codesandbox-ci bot commented Apr 22, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 9d730c4:

Sandbox Source
remeda-example-vanilla Configuration

@eranhirsch eranhirsch changed the title chore(lint): Update eslint to v9 fix: "no-dangle-underscore", and other nits. Apr 22, 2024
@eranhirsch
Copy link
Collaborator Author

It's taking the community longer than I thought to migrate to eslint v9, and the resulting changes in this PR don't need v9 to work - so we can ship the changes to the config itself later.

This mainly removes the underscore-based private functions in favor of proper names and directories.

Also removes runtime protections from sample which was the only function we had with runtime protections.

@eranhirsch eranhirsch marked this pull request as ready for review April 22, 2024 07:27
@eranhirsch eranhirsch merged commit 4da195b into beta Apr 22, 2024
17 checks passed
@eranhirsch eranhirsch deleted the eranhirsch/v2/eslint9 branch April 22, 2024 07:27
@TkDodo
Copy link
Collaborator

TkDodo commented Apr 22, 2024

🎉 This PR is included in version 2.0.0-beta.26 🎉

The release is available on:

Your semantic-release bot 📦🚀

@TkDodo
Copy link
Collaborator

TkDodo commented May 30, 2024

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released on @beta released v2 PRs that are part of the v2 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants