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

Update documentation #237

Merged
merged 5 commits into from Oct 14, 2019

Conversation

@DuaneOBrien
Copy link
Contributor

commented Oct 5, 2019

Clarifies the language around prerequisites and installation
Adds .env.template to make it easier for prospective adopters to install
Adds explicit License stanza to the README

DuaneOBrien added 3 commits Oct 5, 2019
Updating my fork
@now

This comment has been minimized.

Copy link

commented Oct 5, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@codecov-io

This comment has been minimized.

Copy link

commented Oct 5, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #237   +/-   ##
=======================================
  Coverage   49.13%   49.13%           
=======================================
  Files          14       14           
  Lines         232      232           
  Branches       41       41           
=======================================
  Hits          114      114           
  Misses         96       96           
  Partials       22       22

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 ef66660...6074b47. Read the comment docs.

Copy link
Member

left a comment

I originally wanted to avoid requiring the GitHub API Key and Secrets. Back Your Stack can work without it and it's a waste of time for the developer to register the GitHub app if you want to fix something that doesn't require it.

Can we somehow phrase it to make clear it's optional for development purpose?

@DuaneOBrien

This comment has been minimized.

Copy link
Contributor Author

commented Oct 5, 2019

Sure, I can work on clarifying that this evening.

I'm assuming the keys only really matter when there's a risk of the developer hitting rate limiting because BYS is calling out to GitHub frequently. Is this only really an issue when it's crawling large organizations or users with a lot of repos? Are there other scenarios where a developer might run up against the rate limit?

@DuaneOBrien

This comment has been minimized.

Copy link
Contributor Author

commented Oct 6, 2019

Updated the language around the API keys. Let me know if this sounds better.

@znarf
znarf approved these changes Oct 8, 2019
@znarf znarf requested a review from flickz Oct 13, 2019
@znarf

This comment has been minimized.

Copy link
Member

commented Oct 13, 2019

@flickz can you review and merge if everything is good for you?

@flickz
flickz approved these changes Oct 14, 2019
Copy link
Contributor

left a comment

LGTM, thank you @DuaneOBrien

@flickz flickz merged commit 8139ca4 into opencollective:master Oct 14, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
now Deployment has completed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.