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

added couchbase probe for couchnode package #498

Closed
wants to merge 9 commits into from
Closed

added couchbase probe for couchnode package #498

wants to merge 9 commits into from

Conversation

jakewarr8
Copy link

@codecov-io
Copy link

codecov-io commented Nov 3, 2017

Codecov Report

Merging #498 into master will decrease coverage by 0.35%.
The diff coverage is 29.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #498      +/-   ##
==========================================
- Coverage    60.8%   60.45%   -0.36%     
==========================================
  Files          49       50       +1     
  Lines        3215     3252      +37     
==========================================
+ Hits         1955     1966      +11     
- Misses       1260     1286      +26
Impacted Files Coverage Δ
probes/couchbase-probe.js 29.72% <29.72%> (ø)

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 2b66741...99b12a0. Read the comment docs.

@jakewarr8
Copy link
Author

I'm not sure what is making codecov fail.

@tobespc
Copy link
Member

tobespc commented Nov 7, 2017

@astub before we can accept your change request, please can you read and accept

https://github.com/RuntimeTools/appmetrics/blob/master/CONTRIBUTING.md

* bump version for packaging fix

* include omr-agentcore level
@sjanuary
Copy link
Contributor

sjanuary commented Nov 9, 2017

@astub if your test can be run without needing a database to be set up (looks like a mock one?) then you can add it to the list of tests we run in package.json https://github.com/RuntimeTools/appmetrics/blob/master/package.json#L32. We call out the tests individually because we have some probe tests that require a service setting up like a database so we don't run those by default. That would also fix the codecov decrease.

@jakewarr8
Copy link
Author

@sjanuary Hi. It uses couchbase.Mock so it does not need a couchbase server. I'm unsure what else i'm missing for this PR.

@sjanuary
Copy link
Contributor

@astub - you need to rebase as master has changed since this PR and there are conflicts. Also if you rename your test file to end in "tests.js" then it will be run automatically - see https://github.com/RuntimeTools/appmetrics/blob/master/package.json#L32

@jakewarr8
Copy link
Author

@sjanuary Thanks for your feedback. Anything else I need to improve/fix?

@sjanuary
Copy link
Contributor

sjanuary commented Nov 21, 2017

@astub please could you also add a description the couchbase event to the Readme? It needs to go in the table at the start and also here. Also your PR looks a little strange, it seems to have some other people's changes in.

@jakewarr8
Copy link
Author

closing this as its a copy of #506

@jakewarr8 jakewarr8 closed this Nov 22, 2017
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.

6 participants