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

chore: replace traverse with neotraverse #2285

Merged

Conversation

digiz3d
Copy link
Contributor

@digiz3d digiz3d commented Aug 1, 2024

NGL I made this PR just to try it out. Hope it helps

This replaces traverse dependency, which is bloated with lots of useless dependencies, especially since the 0.6.9 version.
Only to support node 0.4 that nobody uses anymore.

You can find many information about it on Twitter/X. Here is an article from the replacement library author here https://puruvj.dev/blog/forking-and-fixing-traverse

Checklist

  • Sign off your commits with DCO (Developer Certificate of Origin)
  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • Commit messages are following our guidelines

@digiz3d digiz3d requested a review from dhmlau as a code owner August 1, 2024 21:33
@digiz3d digiz3d force-pushed the chore/replace-traverse-with-neotraverse branch from 611a549 to 9f855cf Compare August 1, 2024 21:36
@digiz3d digiz3d changed the title chore: replace traverse with noetraverse chore: replace traverse with neotraverse Aug 1, 2024
@dhmlau
Copy link
Member

dhmlau commented Sep 4, 2024

@digiz3d, thanks for the PR. And thanks for including the blog post, it's an interesting read :)
Could you please rebase your branch to resolve the merge conflicts?

@digiz3d digiz3d force-pushed the chore/replace-traverse-with-neotraverse branch from 9f855cf to dccccf4 Compare September 15, 2024 18:28
@digiz3d
Copy link
Contributor Author

digiz3d commented Sep 15, 2024

@digiz3d, thanks for the PR. And thanks for including the blog post, it's an interesting read :) Could you please rebase your branch to resolve the merge conflicts?

Glad you found it interesting! done


Unrelated, but on my current computer (mac M1) tests do not seem to pass, even on master. It looks like it is expecting english but receives french (probably my locale... somewhere?): image

edit: all good ✅ works with STRONGLOOP_GLOBALIZE_APP_LANGUAGE=en npm test

@coveralls
Copy link

coveralls commented Sep 16, 2024

Pull Request Test Coverage Report for Build 10938709817

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 84.715%

Totals Coverage Status
Change from base Build 10935874961: 0.0%
Covered Lines: 7281
Relevant Lines: 8287

💛 - Coveralls

@achrinza
Copy link
Member

Hi @digiz3d, thanks for the pull request! Just need to fix the merge conflict - Could you regenerate the lockfile with npm i --package-lock-only? Then we should be good to merge.

@digiz3d digiz3d force-pushed the chore/replace-traverse-with-neotraverse branch from dccccf4 to 07d37fc Compare September 19, 2024 09:58
@digiz3d
Copy link
Contributor Author

digiz3d commented Sep 19, 2024

Just need to fix the merge conflict

I've rebased the PR, lockfile should be up to date now 👍

@achrinza achrinza merged commit 5e04826 into loopbackio:master Sep 19, 2024
11 checks passed
@achrinza
Copy link
Member

achrinza commented Sep 19, 2024

Thanks, it's been merged 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants