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

Run multiple Dangerfiles #1035

Closed
mokagio opened this issue Apr 28, 2020 · 4 comments
Closed

Run multiple Dangerfiles #1035

mokagio opened this issue Apr 28, 2020 · 4 comments

Comments

@mokagio
Copy link
Contributor

mokagio commented Apr 28, 2020

Something I love about Peril is how you can define rules in the form of Dangerfiles, and run multiple of them under the same instance. E.g. on the WordPress iOS app we run a bunch of different self contained checks, each defined in its own .ts file.

I'm playing around with using Danger JS via GitHub Action instead of a self hosted Peril. I want to keep all the rules granular and in their dedicated repo, so that they can be shared among different projects, as we currently do wit h Peril. The only approach I found so far to run multiple checks is to have multiple instances of Danger, differentiated via the --id option. That works well, but also produces multiple comments.

An alternative way could be to let a single danger ci instance run multiple Dangerfiles. I think this would be possible if the --dangerfile option allowed multiple parameters or if we could pass multiple instances of the option.

yarn run danger ci \
  --dangerfile Automattic/peril-settings/org/pr/label.ts,Automattic/peril-settings/org/pr/ios-macos.ts

or

yarn run danger ci \
  --dangerfile Automattic/peril-settings/org/pr/label.ts \
  --dangerfile Automattic/peril-settings/org/pr/ios-macos.ts

I like the multiple --dangerfile instances, but I wonder if it's confusing and/or whether the way option parsing is currently done would allow for it. A comma separated list is ugly to look at, but would be simpler to manage.

What do you think?

@orta
Copy link
Member

orta commented Apr 28, 2020

Hrm, yeah, I could see that working.

I think it'd maybe require a lot of data plumbing (there's not really a concept of consolidating multiple runs ATM) but conceptually the idea is sound

yarn danger ci --dangerfile   Automattic/peril-settings/org/pr/label.ts  Automattic/peril-settings/org/pr/ios-macos.ts 

There are hacks and a WIP PR for tj/commander.js#571 so that could be possible

@mokagio
Copy link
Contributor Author

mokagio commented Apr 29, 2020

Neat! Thank for the info. I'll keep an eye on that and see where it leads. I may get time in a few weeks to try and make this happen too.

@mokagio
Copy link
Contributor Author

mokagio commented Jun 4, 2020

I have a little update. I realized there is a better way to run multiple Dangerfiles, and that's to have a master Dangerfile importing them. E.g. this:

export async function wordPressIOS () {
  // The imports _need_ to be done within this async function
  // picked up by the imports resolver Danger uses with remote
  const {checkLabel} = await import("../org/label")
  const {checkDiffSize} = await import("../org/pr/diff-size")
  const {iOSSafetyChecks} = await import("../org/pr/ios")

  await checkLabel()
  await checkDiffSize()
  await iOSSafetyChecks()
}

export default wordPressIOS

It took me a bit to realize how to configure TypeScript in order to get the imports to be resolved. Or rather, it took me a while to realize that the Danger plugin template already has it all sorted out and copy it.

This approach does what we need and I think it's actually neater than simply adding more --dangerfile arguments, because the master file could have logic as well to do stuff like only running a subset of the checks depending on the branch or other parameters.

Given this option, I think the idea of supporting multiple --dangerfile loses it's appeal.

@orta
Copy link
Member

orta commented Jun 4, 2020

Yeah, I'm good for this - let's close it 👍

@orta orta closed this as completed Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants