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: fix code coverage, drop ncp #1013

Merged
merged 1 commit into from
May 3, 2019
Merged

chore: fix code coverage, drop ncp #1013

merged 1 commit into from
May 3, 2019

Conversation

kjin
Copy link
Contributor

@kjin kjin commented May 2, 2019

  • ncp causes issues with running scripts locally. It hasn't been updated in 4 years, so I'm dropping it favor of just cp -r cpy.
  • We were writing coverage info to coverage but codecov appears to expect these files to exist in .coverage.
  • I disabled --all as it slows down CI a ton, enough to cause flakes.
  • I added CI Codecov flags. Not 100% sure if it will work until we can view diffs, but this would allow different coverage reports for different Node versions.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 2, 2019
@ofrobots
Copy link
Contributor

ofrobots commented May 2, 2019

cp -r wouldn't work on windows.

@codecov

This comment has been minimized.

@kjin
Copy link
Contributor Author

kjin commented May 2, 2019

@ofrobots ah, thanks for the reminder.

@codecov-io
Copy link

codecov-io commented May 2, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@98f95e3). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1013   +/-   ##
=========================================
  Coverage          ?   91.83%           
=========================================
  Files             ?      107           
  Lines             ?     6380           
  Branches          ?      485           
=========================================
  Hits              ?     5859           
  Misses            ?      382           
  Partials          ?      139

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 98f95e3...a23c33e. Read the comment docs.

@kjin kjin force-pushed the chores branch 3 times, most recently from c9b9ea6 to 251b036 Compare May 2, 2019 22:34
@kjin kjin requested a review from a team May 2, 2019 23:27
@kjin kjin marked this pull request as ready for review May 2, 2019 23:27
@ofrobots
Copy link
Contributor

ofrobots commented May 2, 2019

For posterity, what are the problems that ncp is causing locally?

@kjin
Copy link
Contributor Author

kjin commented May 3, 2019

ncp has an issue where sometimes a callback isn't called. Since I promisify it that means a Promise won't resolve, and my build script never continues -- it just exits with error code 0 (which IMO is the worst part). AvianFlu/ncp#131 is an example, their issue list seems to suggest several such problems.

The repo and module seem unmaintained, so I don't have hope that this will be fixed.

@kjin kjin merged commit 6b7f923 into googleapis:master May 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants