Skip to content
This repository has been archived by the owner on Oct 17, 2023. It is now read-only.

docs: generate new samples/README.md, README.md #268

Merged
merged 9 commits into from
May 20, 2019
Merged

docs: generate new samples/README.md, README.md #268

merged 9 commits into from
May 20, 2019

Conversation

bcoe
Copy link
Contributor

@bcoe bcoe commented May 17, 2019

switch to new auto-generated README and samples/README.md.

@bcoe bcoe requested a review from leahecole May 17, 2019 21:29
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 17, 2019
@codecov
Copy link

codecov bot commented May 17, 2019 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #268   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines           3      3           
=====================================
  Hits            3      3

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 1d3d216...cff33d4. Read the comment docs.

JustinBeckwith
JustinBeckwith suggested changes May 17, 2019
View reviewed changes
.repo-metadata.json Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
samples/README.md Show resolved Hide resolved
@bcoe bcoe mentioned this pull request May 17, 2019
Merged
@bcoe bcoe mentioned this pull request May 17, 2019
Closed
3 tasks
@leahecole leahecole mentioned this pull request May 17, 2019
Closed
leahecole
leahecole reviewed May 17, 2019
View reviewed changes
Copy link
Contributor

@leahecole leahecole left a comment

Choose a reason for hiding this comment

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

Other than that small nit wanting to make sure the version change is okay, LGTM

If a yoshi expert deems it good, I trust them :)

synth.metadata Outdated Show resolved Hide resolved
JustinBeckwith
JustinBeckwith suggested changes May 17, 2019
View reviewed changes
samples/quickstart.js Outdated
@@ -16,7 +16,7 @@
'use strict';

// [START translate_quickstart]
Copy link
Contributor

Choose a reason for hiding this comment

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

The region tag here should be moved right before the // Imports the Google Cloud... Inside of the main function, after we instantiate the client, create an async quickstart method that takes no params. Then call it!

.readme-partials.yml
npm install
```

#### Run the Tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Silly question - why are we telling developers how to run the sample tests in the README? That feels like something for CONTRIBUTING at best.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not silly - is that where we're putting them in other repos? My argument for leaving the command in the README (perhaps in addition to CONTRIBUTING?) is that not all users are contributors, and I see it as a teaching tool to promote TDD

@leahecole leahecole merged commit 60799aa into master May 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@leahecole leahecole leahecole left review comments

@JustinBeckwith JustinBeckwith JustinBeckwith approved these changes

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bcoe @JustinBeckwith @leahecole @googlebot