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

Add snippets to the admin repo #130

Merged
merged 6 commits into from
Feb 20, 2018
Merged

Add snippets to the admin repo #130

merged 6 commits into from
Feb 20, 2018

Conversation

samtstern
Copy link
Contributor

They are linted by lint.sh, not sure if that fails below a certain score?

@hiranya911
Copy link
Contributor

hiranya911 commented Feb 17, 2018

The linter fails the build if it detects at least one error. But looks like the linter passed on this one, which is good. I see the following in the last Travis build log:

$ if [[ "$PY_VERSION" == '2' ]]; then ./lint.sh all; fi
Running linter on module firebase_admin
Running linter on module tests
Running linter on module integration
Running linter on module snippets

We have more snippets at https://github.com/firebase/quickstart-python. I think we should move them here as well.

@samtstern
Copy link
Contributor Author

@hiranya911 I added all the relevant quickstart-python snippets here and also moved the files around to be in subfolders. Lint gets 10/10 now.

@samtstern
Copy link
Contributor Author

However the 581a9dc should have failed due to lint, and it did not...

@samtstern
Copy link
Contributor Author

I'll wait for @hiranya911 to merge.

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

LGTM.

Nit: Some files are missing the license header.

@samtstern samtstern merged commit 432a207 into master Feb 20, 2018
carsongee pushed a commit to carsongee/firebase-admin-python that referenced this pull request Mar 13, 2018
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.

None yet

3 participants