danger-js

danger-js copied to clipboard

TL;DR I might be doing something wrong, but I can get Danger to run "properly" in GitHub Actions workflows that are triggered by events other than PRs.

There's two possibilities:

• It's not meant to work for events other than PRs. If so, how can it be made clearer?
• It should work. If so, what am I doing wrong / what's wrong in the code?

---

I've been working on migrating our Peril setup to Danger + GitHub Actions on an off recently.

Danger works great for PR, but when it comes to other type of events that can trigger a GitHub Action workflow, it's not as effective.

For example, in this repo I set up a workflow to run on the status event and told it to post some messages.

No messages were posted. That's not that surprising, actually. How can Danger know that it's dealing with a status event of a commit that's part of a PR? Looking at the GitHubActions.ts CI source confirms my suspicion. In particular:

get useEventDSL() {
  return this.event.pull_request === undefined && this.event.issue === undefined
}

As far as I understand in the code, if useEventDSL() returns true, Danger will use the "simple DSL", which is... sort of empty?

// When there's an event we don't need any of ^
getPlatformReviewSimpleRepresentation: async () => ({}),

The Dangerfile I run in my test workflow dumps the danger object, and this is what it prints

{
    "git": {},
    "github": {
        "api": {
            "actions": {},
            "activity": {},
            "apps": {},
            "checks": {},
            "codesOfConduct": {},
            "emojis": {},
            "gists": {},
            "git": {},
            "gitignore": {},
            "interactions": {},
            "issues": {},
            "licenses": {},
            "log": {},
            "markdown": {},
            "meta": {},
            "migrations": {},
            "oauthAuthorizations": {},
            "orgs": {},
            "projects": {},
            "pulls": {},
            "rateLimit": {},
            "reactions": {},
            "repos": {},
            "search": {},
            "teams": {},
            "users": {}
        },
        "utils": {}
    },
    "utils": {}
}

Another example is the behavior with issues events, see #999.

Am I missing something? If so, then how I can get more information out of danger when running from an event that's not a PR. Otherwise, there's two options I see here:

• Danger's tag line is "Stop saying 'you forgot to …' in code review", it's fine that it's not that useful for other events, there's Peril for that; or
• Given Peril is in a sort of maintenance mode, it makes sense to try and source more information from the GitHub context of events other than PRs when run via GitHub Actions. If so, how would we go about it? (It also sounds like a lot of work)

I added the random event support during the GH Actions alpha to give it a test run, and to be able to have some of my dangerfiles ported over from Peril and it mostly did what I wanted. This pre-dates GH actions running JS natively, so I was mostly just using Danger to give myself a consistent TS runtime with access to the GH API.

I think the github.api does work though, the output of dumping it might just have getters instead of hardcoded fields and so they may not show up

I'm open to it being more useful, for example github.event could be a thing which gives you the JSON dump, but I'm not sure about forcing every event to reach back to a PR at system level. A status is intentionally pretty decoupled from a PR by GH and you gotta put in work (via the search API) to find it (and it could return more than 1 PR for example) - so I'm not sure it's danger's responsibility to hook that up

When I did https://github.com/danger/danger-js/pull/940 my idea was exactly to extend Danger to all the PR related cases. I agree with Orta that status is not strictly a PR related event, more a commit related event, there could in fact not be a related PR. What about allowing some env variables in order to be able to still run Danger in Actions on those cases? Then doesn't matter what the actual event is, if is not a PR related one and you can retrieve the actual PR data you can still pass that to Danger

Thanks @orta and @f-meloni for the feedback. 😄 🙏

I think the github.api does work though, the output of dumping it might just have getters instead of hardcoded fields and so they may not show up

I didn't think of that 😳 . I'm a JS/TS noob.

I'm not sure about forcing every event to reach back to a PR at system level. A status is intentionally pretty decoupled from a PR by GH and you gotta put in work (via the search API) to find it (and it could return more than 1 PR for example) - so I'm not sure it's danger's responsibility to hook that up

+1. I really like the "Stop saying 'you forgot to …' in code review" tagline. Trying to make too much of a general purpose tool would be a slippery slope of complexity

What about allowing some env variables in order to be able to still run Danger in Actions on those cases? Then doesn't matter what the actual event is, if is not a PR related one and you can retrieve the actual PR data you can still pass that to Danger

That sounds like a great place to start. It might be there's more that can be done in the current setup, too, and it's just that I don't know how to read some of those information. I'll play around with that a bit.