search

apache / cordova-ios

Apache Cordova iOS
https://cordova.apache.org/
Apache License 2.0
2.16k stars 989 forks source link

refactor: hide implementations of build & run #1188

Closed raphinesse closed 2 years ago

raphinesse commented 2 years ago

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-commenter commented 2 years ago

Codecov Report

Merging #1188 (c6665e5) into master (ea9751b) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1188   +/-   ##
=======================================
  Coverage   75.03%   75.03%           
=======================================
  Files          13       13           
  Lines        1650     1650           
=======================================
  Hits         1238     1238           
  Misses        412      412
Impacted Files Coverage Δ
bin/templates/scripts/cordova/lib/build.js 44.09% <ø> (ø)
bin/templates/scripts/cordova/lib/run.js 22.22% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ea9751b...c6665e5. Read the comment docs.

raphinesse commented 2 years ago

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.

erisu commented 2 years ago

I am OK with any label, but this should be good to merge in.