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

document GCS authentication #415

Merged
merged 3 commits into from
Jan 30, 2020
Merged

Conversation

petedannemann
Copy link
Contributor

@petedannemann petedannemann commented Jan 28, 2020

document GCS authentication

Please pick a concise, informative and complete title for your PR.
The title is important because it will appear in our change log.

Motivation

Please explain the motivation behind this PR in the description.

If you're fixing a bug, link to the issue number like so:

  • Add missing docs based on discussion here

If you're adding a new feature, then consider opening a ticket and discussing it with the maintainers before you actually do the hard work.

Tests

If you're fixing a bug, consider test-driven development:

  1. Create a unit test that demonstrates the bug. The test should fail.
  2. Implement your bug fix.
  3. The test you created should now pass.

If you're implementing a new feature, include unit tests for it.

Make sure all existing unit tests pass.
You can run them locally using:

pytest smart_open

If there are any failures, please fix them before creating the PR (or mark it as WIP, see below).

Work in progress

If you're still working on your PR, include "WIP" in the title.
We'll skip reviewing it for the time being.
Once you're ready to review, remove the "WIP" from the title, and ping one of the maintainers (e.g. mpenkov).

Checklist

Before you create the PR, please make sure you have:

  • Picked a concise, informative and complete title
  • Clearly explained the motivation behind the PR
  • Linked to any existing issues that your PR will be solving
  • Included tests for any new functionality
  • Checked that all unit tests pass

@petedannemann petedannemann requested a review from mpenkov January 28, 2020 22:01
@petedannemann petedannemann changed the title Add GCS authentication documentation to README document GCS authentication Jan 29, 2020
Copy link
Collaborator

@mpenkov mpenkov left a comment

Choose a reason for hiding this comment

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

Good start. Left you some comments. Please check them out.

@petedannemann
Copy link
Contributor Author

@mpenkov thanks for the review. I addressed your comments

Copy link
Collaborator

@mpenkov mpenkov left a comment

Choose a reason for hiding this comment

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

Very good. Could you please add links to Google's documentation about where you can get this JSON and token? It would be a great help to people unfamiliar with GCS, like me, when they first try to play around with it.

@petedannemann
Copy link
Contributor Author

Sure thing, I just added that to the docs.

Copy link
Collaborator

@mpenkov mpenkov left a comment

Choose a reason for hiding this comment

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

Very good. Thank you!

@mpenkov mpenkov merged commit 611cbfb into piskvorky:master Jan 30, 2020
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.

2 participants