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

Don't run danger if DANGER_GITHUB_API_TOKEN is not set #1276

Merged
merged 2 commits into from
Jun 14, 2017

Conversation

shilman
Copy link
Member

@shilman shilman commented Jun 14, 2017

Issue: #1275

What I did

Don't run danger if DANGER_GITHUB_API_TOKEN is not set

How to test

@shilman shilman requested review from ndelangen and orta June 14, 2017 10:49
@orta
Copy link
Member

orta commented Jun 14, 2017

This means anyone making a PR from a fork won't have danger run for them, it's your call if that's what you're looking for. However, I'd recommend making the bot have no access and making the token non-secret on CI

@codecov
Copy link

codecov bot commented Jun 14, 2017

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1276   +/-   ##
=======================================
  Coverage   13.73%   13.73%           
=======================================
  Files         207      207           
  Lines        4638     4638           
  Branches      510      519    +9     
=======================================
  Hits          637      637           
- Misses       3550     3551    +1     
+ Partials      451      450    -1
Impacted Files Coverage Δ
.../src/manager/containers/CommentsPanel/dataStore.js 34.78% <0%> (ø) ⬆️
addons/info/src/components/markdown/htags.js 0% <0%> (ø) ⬆️
lib/ui/src/modules/ui/containers/left_panel.js 20% <0%> (ø) ⬆️
lib/ui/src/modules/ui/containers/layout.js 12.5% <0%> (ø) ⬆️
addons/knobs/src/components/types/Boolean.js 11.62% <0%> (ø) ⬆️
...rc/modules/ui/components/left_panel/text_filter.js 33.33% <0%> (ø) ⬆️
.../ui/src/modules/ui/components/layout/dimensions.js 15.62% <0%> (ø) ⬆️
app/react/src/client/preview/init.js 0% <0%> (ø) ⬆️
lib/ui/src/modules/ui/configs/init_panels.js 25% <0%> (ø) ⬆️
addons/knobs/src/components/PropField.js 10.86% <0%> (ø) ⬆️
... and 22 more

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 36d7ec2...8ec814f. Read the comment docs.

@shilman shilman added the cleanup Minor cleanup style change that won't show up in release changelog label Jun 14, 2017
@shilman shilman merged commit 606f34b into master Jun 14, 2017
@ndelangen ndelangen deleted the danger-check-pr-labels branch June 14, 2017 22:19
@orta
Copy link
Member

orta commented Jun 15, 2017

I had a think about this, you could create two tokens on your CI a private DANGER_GITHUB_API_TOKEN, which has access to the status API, and a DANGER_GITHUB_API_TOKEN_OPEN which has no access, that you switch at runtime if DANGER_GITHUB_API_TOKEN doesn't exist.

@orta
Copy link
Member

orta commented Jun 15, 2017

I'd also 👍 a PR adding support in Danger for first looking at DANGER_GITHUB_API_TOKEN, then checking DANGER_GITHUB_API_TOKEN_OPEN so that you have to do no work on CI there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Minor cleanup style change that won't show up in release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants