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

Check labels PR using Danger JS via GitHub action #4

Closed
wants to merge 13 commits into from

Conversation

mokagio
Copy link
Owner

@mokagio mokagio commented Feb 7, 2020

As described in #1, since I can't get fast feedback on how Danger interacts with GitHub issues, I'm moving to attempt to integrate Danger in the PR process.

This is all aimed at addressing wordpress-mobile#13366.

Here, we add a GitHub action that runs the shared Peril setting to leave warnings on PRs without labels.

To test this, I'll be adding and removing labels to PRs. If everything works, Danger will post warnings in those PRs via a bot account.

It works on my machine:

Screen Shot 2020-02-07 at 13 15 28

We'll use this to eventually run Danger. At the moment, we just want to
make sure the workflow runs on the given hooks.
First step to implement wordpress-mobile#13366.

The idea is to start with something simple, that is commenting on issues that don't have a label, replicating this Peril config: https://github.com/mokagio/peril-settings/blob/7a7d11f69351d446b8e3a50d9e14a7fdf43a21e4/peril-settings.json#L17-L19.

With this into the main branch of the repo, opening, labeling, and un-labeling an issue should result in a workflow run.
I like the standalone setup. This way, we wouldn't need to manage yarn or even 
add rules to WordPress mobile repo.

I do wonder though if it'd be possible to version the file to run, so that we 
only upgrade it intentionally. In other words, I'd like to avoid a change in 
the rule made for the sake of one repo breaking the behavior on other repos.

🤔... I guess this is a tradeoff to explore, is centralized management worth 
the risk?
@mokagio-bot
Copy link

Warnings
⚠️ PR is missing at least one label.

Generated by 🚫 dangerJS against 1e25570

@mokagio
Copy link
Owner Author

mokagio commented Feb 7, 2020

Works 🎉

Screen Shot 2020-02-07 at 13 16 58

@mokagio mokagio added the enhancement New feature or request label Feb 7, 2020
@mokagio
Copy link
Owner Author

mokagio commented Feb 7, 2020

Adding the label triggered a new workflow run

Screen Shot 2020-02-07 at 13 17 38

@mokagio
Copy link
Owner Author

mokagio commented Feb 7, 2020

Weird... The build said it would have removed existing messages, but the warning is still here:

screencapture-github-mokagio-WordPress-iOS-pull-4-2020-02-07-13_21_25 Screen Shot 2020-02-07 at 13 26 56

The main reason for this dedicated commit is to trigger a new build in
CI and see if Danger removes the warning regarding not having any
labels.
@mokagio
Copy link
Owner Author

mokagio commented Feb 7, 2020

Same as before 😞 . Build passed but warning still in the PR.

screencapture-github-mokagio-WordPress-iOS-pull-4-2020-02-07-13_32_30

@mokagio-bot
Copy link

Fails
🚫 Podfile: reference to a commit hash

Generated by 🚫 dangerJS against 0bb337a

@mokagio
Copy link
Owner Author

mokagio commented Feb 7, 2020

Made a new experiment, to see it a build failure is removed.

First step, actually make the build fail:

Screen Shot 2020-02-07 at 13 52 23

@mokagio
Copy link
Owner Author

mokagio commented Feb 7, 2020

😞 , yet another message not removed, despite the build saying so

Screen Shot 2020-02-07 at 13 57 37

At this point, I wonder if there's a setting I'm missing somewhere.

@mokagio mokagio added enhancement New feature or request and removed enhancement New feature or request labels Feb 11, 2020
@mokagio
Copy link
Owner Author

mokagio commented Feb 11, 2020

Closing in favor of #6.

@mokagio mokagio closed this Feb 11, 2020
@mokagio mokagio deleted the danger-pr-integration branch February 12, 2020 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants