Danger: Proposal: Major changes to `danger local`

[orta]

lol @ this being a major SemVer change already. This is Danger 4.0, as command line changes like this is a major release.

These https://github.com/danger/danger/issues/381 https://github.com/danger/danger/issues/370 https://github.com/danger/danger/issues/368 issues all touch on the current cognitive dissonance around what you expect danger local to be and what it is.

After talking with @dbgrandi and @dblock - I think I've come to a conclusion.

• danger local- runs the current changes since the last commit in Danger against the current Dangerfile.
• danger pr [pr] - runs the given PR against the current Dangerfile. This means you can PR against any PR, regardless of forks in an intuitive way.

---

The hardest thing here is danger local will need to be able to handle the ambiguity of _not_ having a PR to work against. Something @justMaku brought up here https://github.com/danger/danger/pull/299#issuecomment-231165513 - so good on you for being three weeks ahead of me.

I'm not 100% on the ideal here actually I see one of two patterns:

• Stub out with example data
• don't initialize a request source

This pattern will need to work with the work in #299 on supporting multiple request sources too. So maybe pulling in the commits @k0nserv mentioned here https://github.com/danger/danger/pull/299#issuecomment-235624279 is a pre-requisite to doing this work. Wherein we define a Random request source, ideally one that submits data based on Danger (the app)'s music influences.

[marcelofabri]

👍 for improving danger local. Another thing that I'd add is making sure to support forks as well (in case of danger pr).

[orta]

the [pr] bit would be danger pr https://github.com/danger/danger/pull/362 so it can have any URL, which can then be used to support gitlab too

[marcelofabri]

Awesome, I assumed that it was the PR number

[ashfurrow]

It'd be cool to have an option to run danger against the diff from the current branch and master.

[justMaku]

Since I was thinking about this recently I believe ideal solution to the problem with danger local would be to drastically change Danger's architecture.

Here's a concept I have in mind:

• "Raw" danger executes a Dangerfile on a specified changeset: danger master HEAD. It returns the output using standard output.
• We have a new entity called OutputSomething (can't decide what to call it) which handles all of the PR updating staff we do in the RequestSource now.
• RequestSource continues to be used as a way to get more information about the changeset (base commit, etc).
• CISource job would be to detect the changeset and an output source (GitHub, GitLab, BitBucket, Slack, Twitter, you name it)
• EnvironmentManager ties it all together.

I have started working on it but had to put it off for now due to personal stuff happening. Let me know what you think.

Having this architecture would solve problems like #379 where you could add Danger without a CI to your project using a GitHub hooks etc.

[orta]

Ah, I see, yeah, so that Danger is effectively a multi-step process, this can make it much easier to test too. I grok it. Makes sense 👍

[mtitolo]

I do like the idea of being able to run this against local changes. But this does end up losing context.

For instance, I have a rule that checks the pr_body for screenshots whenever a storyboard or xib is modified, and fails when there are none. I don't want to maintain separate Dangerfiles for running locally vs as a PR. How would those kinds of differences work with the new system?

Also, I'm behind a firewall and anything that requires the command line to talk to the internet is difficult. To get Danger working I already need a command line proxy running. So I'm 👎 for using raw HTTP links to run Danger locally.

[orta]

How would those kinds of differences work with the new system?

I'm still not sure of this, I definitely don't want to have separate Dangerfiles. So far, I've only come up with putting dummy data ( or allowing you to say "use this PR for that" ) - as it's a new feature, it really can look like anything it wants.

Also, I'm behind a firewall and anything that requires the command line to talk to the internet is difficult. To get Danger working I already need a command line proxy running. So I'm 👎 for using raw HTTP links to run Danger locally.

This is about making it more explicit, currently it just infers that url from your repo but under the hood it still generates a http url to work with. With an actual url you can define other git repos too. It'll still have to use the git commands under the hood to compare data

[orta]

Idea: The danger object should contain a bool about whether it is running in local or not.

Could make it more reasonable to do something like:

unless danger.local
  fail("Needs MR title") unless gitlab.mr_title.length > 5
end

credit to @mtitolo

[mtitolo]

It's almost as if people need to be able to setup danger workflows. We'd need at least 4 different Dangerfile versions, due to a complicated git process due to a large team. While we could have multiple Dangerfiles in different branches/forks, it'll get difficult to run locally when you need to use a target's Dangerfile instead of the copy you have.

I seem to only complicate things >.<

[iainsmith]

What do people think about reusing the concept of lanes from Fastlane? If no explicit lanes are defined, the Danger file works as it does today. When lanes are defined they need to be called explicitly.

Specifically this would be great when you want a pre push hook to run some simple (and fast) checks against your current branch and master.

[marcelofabri]

I like the lanes idea 👍

We basically do this today with multiple Dangerfiles and danger_ids

[orta]

I scoped out a few months ago a way so we could handle different inputs from GitHub/GitLab etc - https://github.com/danger/danger/issues/42#issuecomment-219752336

Could be extended to provide scopes for Danger, aye

[urkle]

Will this overhaul of danger local also support being able to run a danger file against local commits (not pushed).. as while I'm developing my dangerfile I need to test on scenarios of commits and being able to run it all local would be really nice. IMHO something that fakes a PR and takes current HEAD and finds the merge-base against master and uses that range to do the git range for the danger file would be awesome.

Also removing the assumption in danger local that everyone uses the github UI to do PR merges would be great too.. as we don't use the github UI and instead we rebase+merge thus danger local is 100% useless to us right now.

[carmanchris31]

This sounds like it would be a natural stepping stone to resolving the frustrating "Not a Pull Request" issue?

Danger has the potential to be much more versatile with this, as there are various use cases for using Danger outside of PRs and things like the Github Checks api can be used before a PR is created also.

This sounds really great, particularly with decoupling Danger from PRs. 🎉

[orta]

All of this work happened on Danger JS instead, where it can do everything from pre-commit hooks, non-PR GH actions, GH checks etc