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: add custom documentation to the README #313

Merged
merged 7 commits into from
Feb 12, 2019

Conversation

DominicKramer
Copy link
Contributor

No description provided.

@DominicKramer DominicKramer requested a review from a team February 12, 2019 20:05
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 12, 2019
@DominicKramer DominicKramer added this to Needs review in node-team February BugBash Feb 12, 2019
Copy link
Contributor

@ofrobots ofrobots left a comment

Choose a reason for hiding this comment

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

LGTM w/ nit, once docs CI (broken link) is fixed.

README.md Outdated
@@ -83,6 +116,247 @@ has instructions for running the samples.
| --------------------------- | --------------------------------- |
| Examples | [source code](https://github.com/googleapis/nodejs-error-reporting/blob/master/samples/snippets.js) |

## Configuration

The following code snippet lists all available configuration options. All configuration options are optional.
Copy link
Contributor

Choose a reason for hiding this comment

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

all is probably too strong. I think there are other options that the Service parent class accepts that we simply pass on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch.

Copy link
Contributor

@ofrobots ofrobots left a comment

Choose a reason for hiding this comment

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

Given this comment from @JustinBeckwith, please get his LGTM before landing: #190 (comment)

@codecov
Copy link

codecov bot commented Feb 12, 2019

Codecov Report

Merging #313 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #313   +/-   ##
======================================
  Coverage    96.1%   96.1%           
======================================
  Files          34      34           
  Lines        1695    1695           
  Branches       76      76           
======================================
  Hits         1629    1629           
  Misses         49      49           
  Partials       17      17

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 535b265...29ad522. Read the comment docs.

@JustinBeckwith
Copy link
Contributor

It's all good. Thanks for sticking with me on this one. We can re-eval this when we need to do the fancy README gen tool.

@JustinBeckwith JustinBeckwith merged commit 28389fc into googleapis:master Feb 12, 2019
node-team February BugBash automation moved this from Needs review to Done Feb 12, 2019
@DominicKramer
Copy link
Contributor Author

@JustinBeckwith Awesome thanks.

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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants