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

add in handling for PR milestoned/demilestoned events #343

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

urkle
Copy link
Contributor

@urkle urkle commented Aug 1, 2018

Github sends down the milestoned/unmilestoned (setting/unsetting the milestone) events as issue events even for pull requests. Thus they do not get mapped correctly to be processed as a PR change in Peril

@@ -177,6 +177,7 @@ export const runEverything = async (
const eventRuns = runs.filter(r => r.dslType === RunType.import)

// Loop through all PRs, which are require difference DSL logic compared to simple GH webhook events
// TODO a re-mapped PR from an issue event won't have pr.head so how do we handle that in this method?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orta so the issue with my approach here is that the payload from github on the event does not contain the ".head" node. This would be the same issue I'll run into for issue #342 as well. Is there a means to do a "blind" trigger of a PR in peril?

@orta
Copy link
Member

orta commented Aug 3, 2018

Hey Urkle, I'm a little wary of this PR - it disconnects peril's behavior from the GitHub docs, which is something I've been really trying not to do. Mainly that only the PR events get the Danger PR DSL, and everything else is just the event json

So, I'd rather not accept it, though the node version definitely make sense - I default to 8 personally so I always forget to switch. I can extract that commit and merge it into master?

@urkle
Copy link
Contributor Author

urkle commented Aug 4, 2018

@orta the issue is.. the milestoned event can be a pull request. they just incorrectly send it always as a "issues" event. (I have reported this to them but they have not responded back and I doubt they'll fix it). Maybe as some pre-processor that translates the specific "bad" events?

so the problem I'm having is that I need these milestoned/demilestoned events to trigger processing on my pull requests.

I can extract the node 8 out to a separate PR.

So I'm open to suggestions on how I can teach Peril to handle this in a clean way. Right now the main issue is we have to perform some "other event" on the PR to get peril to run again (e.g. change a label, or add a comments) because it won't run for the bad milestoned events from GitHub.

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

Successfully merging this pull request may close these issues.

2 participants