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

Support using the Danger shared GitHub App for OSS repos to work around the forks issue #1126

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

orta
Copy link
Member

@orta orta commented Mar 17, 2021

RE: #1125

Goal: How can we simplify install instructions to as little as possible

@danger danger deleted a comment from danger-oss bot Mar 17, 2021
@orta
Copy link
Member Author

orta commented Mar 17, 2021

OK, confirmed that this isn't reasonably possible as it is. The access to post a comment to the PR requires write access to any PR, and no-one should accept that as a reasonable trade-off for some setup convenience.

Perhaps using Checks is OK, but I've found every time I've used that system to be underwhelming.

@orta
Copy link
Member Author

orta commented Mar 17, 2021

My key argument against checks is that you lose the middle ground: danger gives message / warn / fail - but you can only really use fail in a check because all feedback is hidden from you until you click though to the checks page.

You'd only click if you were blocked.

@orta
Copy link
Member Author

orta commented Mar 17, 2021

Summary: this idea generally works for most of the route

  • Setting up the auth token mostly invisibly (you'd need to follow the instructions to ge the install id but that's easy)
  • Reading all the meta from the PR looks fine with ^
  • Just leaving feedback blocked by the security model

@orta
Copy link
Member Author

orta commented Mar 17, 2021

Took a walk on this

Removing config

The only way this could realistically work is by making some sort of web service, which I'm really not super wild on. The web service could link a repo/org to installation ID (maybe as a GH repo) which danger can look up at runtime. That would eliminate the need for DANGER_GITHUB_APP_INSTALL_ID.

Not using checks

  • Web service: proxy GH API calls for editing/updating/creating comments to its own account which can use a normal API token to post comments.

  • Embed a GH token in danger-js in the same way we embed the private key for GH apps right now

@Jimimaku
Copy link

RE: #1125

Goal: How can we simplify install instructions to as little as possible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants