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

[legacy-framework] refactor(server): simplify resolveBinAsync #208

Merged
merged 1 commit into from
Apr 24, 2020

Conversation

wKovacs64
Copy link
Collaborator

Pull Request Type

  • Feature
  • Bug fix
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Tests related changes
  • Other (please describe): Dependency updates

Checklist

  • Tests added for changes (or N/A)
  • Any added terminal logging uses packages/server/src/log.ts (or N/A)
  • Code Coverage stayed the same or increased

What's the reason for the change? ❓

This is likely a prerequisite for blitz-js/legacy-framework#840 (find-parent-dir is ancient and bombed during some preliminary virtual file system experimentation). It also improves readability (IMHO of course 😉).

What are the changes and their implications? ⚙️

Refactored resolveBinAsync implementation using pkg-dir instead of find-parent-dir. Function signature remained unchanged.

Does this introduce a breaking change? ⚠️

  • Yes
  • No

Other information

N/A

Copy link
Collaborator

@ryardley ryardley left a comment

Choose a reason for hiding this comment

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

lgtm 🚀

Copy link
Member

@flybayer flybayer left a comment

Choose a reason for hiding this comment

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

Thanks!

@flybayer flybayer merged commit ff5cab0 into canary Apr 24, 2020
@flybayer flybayer deleted the refactor-resolveBinAsync branch April 24, 2020 03:45
@itsdillon itsdillon changed the title refactor(server): simplify resolveBinAsync [legacy-framework] refactor(server): simplify resolveBinAsync Jul 7, 2022
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.

3 participants