-
Notifications
You must be signed in to change notification settings - Fork 986
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
refactor: hide implementations of build & run #1188
Conversation
This makes build and run modules more closed by avoiding to expose internals via the resolved promise value. Both modules' main exports previously returned a promise that resolved to a subrocess spawn result. Now these promises resolve to undefined. The public API methods that return these promises did not document a specific promise return value, just when the promise would resolve or reject. Thus, this change is not breaking.
Codecov Report
@@ Coverage Diff @@
## master #1188 +/- ##
=======================================
Coverage 75.03% 75.03%
=======================================
Files 13 13
Lines 1650 1650
=======================================
Hits 1238 1238
Misses 412 412
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be ok... I dont know if anyone wrote wrapping tools that used it...
I'd argue they relied on implementation details. But I'd be totally OK with labeling this change as breaking, since we are targeting a new major anyways. |
I am OK with any label, but this should be good to merge in. |
This makes build and run modules more closed by avoiding to expose internals via the resolved promise value. Both modules' main exports previously returned a promise that resolved to a subrocess spawn result. Now these promises resolve to undefined. The public API methods that return these promises did not document a specific promise return value, just when the promise would resolve or reject. Thus, this change is not breaking.
This makes build and run modules more closed by avoiding to expose
internals via the resolved promise value. Both modules' main exports
previously returned a promise that resolved to a subrocess spawn result.
Now these promises resolve to undefined.
The public API methods that return these promises did not document a
specific promise return value, just when the promise would resolve or
reject. Thus, this change is not breaking.