Skip to content
This repository has been archived by the owner on Jul 28, 2021. It is now read-only.

make fs operations optimistic #24

Merged
merged 2 commits into from
Nov 11, 2018
Merged

make fs operations optimistic #24

merged 2 commits into from
Nov 11, 2018

Conversation

suchipi
Copy link
Contributor

@suchipi suchipi commented Nov 11, 2018

This changes the fs functions to try the normal impl first before calling pkglock.resolve.

It looks like you went ahead and did some of these already over the last few days- I didn't know you wanted them done sooner than the weekend or I wouldn't have volunteered to do them, sorry to keep you waiting.

The merge conflicts may not have been resolved properly because of that; I've checked Allow edits from maintainers so please do whatever you feel makes the most sense.

This changes the fs functions to try the normal impl first before calling pkglock.resolve.
Copy link
Contributor

@zkat zkat left a comment

Choose a reason for hiding this comment

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

nice! You even fixed some bugs in the hacky versions I did while debugging a different thing! Sorry for all the conflicts. I was trying to get another thing working, and you may have fixed it lol

@zkat zkat merged commit bbe8c2e into npm:latest Nov 11, 2018
@zkat
Copy link
Contributor

zkat commented Nov 11, 2018

YOU DID FIX IT. THANK YOU!

#nailedit

@suchipi
Copy link
Contributor Author

suchipi commented Nov 11, 2018

I hope I didn't do too much damage 😉

@suchipi
Copy link
Contributor Author

suchipi commented Nov 11, 2018

oh nice!

@zkat
Copy link
Contributor

zkat commented Nov 11, 2018

omg this fixed one thing while breaking another. For some reason, I've started getting EEXIST errors when trying to initial-load a dep (from an empty cache). I don't know which of the changes is causing this but I guess I'll have to comb through them one-by-one. Really about time we have unit tests for this. sigh.

@zkat
Copy link
Contributor

zkat commented Nov 11, 2018

I managed to fix a few other bits too, at least.

zkat pushed a commit that referenced this pull request Nov 11, 2018
This changes the fs functions to try the normal impl first before calling pkglock.resolve.
@suchipi
Copy link
Contributor Author

suchipi commented Nov 11, 2018

sorry about that. I wasn't sure how to test it

@zkat
Copy link
Contributor

zkat commented Nov 11, 2018

I found the bugs! Everything is good now. I'll stub out some tests soon so folks can help with testing this 👍

@tchalvak
Copy link

+1 for unit tests

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

Successfully merging this pull request may close these issues.

3 participants