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

Adds less severity for WIP and Draft pull requests #62

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

Conversation

firedev
Copy link

@firedev firedev commented May 26, 2023

Let's see if this turns at least drafts green
@firedev firedev requested review from urkle and ayarotsky May 26, 2023 18:38
Copy link
Contributor

@urkle urkle left a comment

Choose a reason for hiding this comment

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

At some point we need to move calling danger to a Github action to run this script and stop using Peril (the web service) as

  1. this will give us more options in danger.
  2. we should be able to replace ALL functionality that the "ruby" dangerfile is doing and put it all in this one file
  3. have milestone and demilestoned events actually work (they do not work in Peril due to Github webhooks being incorrect and Github refusing to fix it)
  4. the developer of danger is no longer maintaining peril since his employer (microsoft) won't let him run Peril on their github account so all his effert is focused on a github actions run of danger.

@firedev
Copy link
Author

firedev commented May 30, 2023

@urkle I just want to be able to get green notification if my code is correct.

@urkle
Copy link
Contributor

urkle commented May 30, 2023

@urkle I just want to be able to get green notification if my code is correct.

You code looks fine, which I approved. We should, however, log a task to move this from Peril to a GH action.

@ayarotsky
Copy link
Member

@firedev Please ping #release-managers to release an update.

Comment on lines -10 to +14
if (labels.includes('work in progress') || danger.git.commits.find(c => c && c.message.includes('WIP'))) {
fail("PR is a Work in Progress");
failNonDraft("PR is a Work in Progress");
Copy link
Contributor

Choose a reason for hiding this comment

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

@ayarotsky @urkle
Won't this change result in a warn or a fail all the time?

The original check was to fail only IF it was a WIP, but this changes unconditionally will generate either a fail(msg) or a warn(msg)

Copy link
Author

@firedev firedev Jan 12, 2024

Choose a reason for hiding this comment

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

@eakoli @ayarotsky We will release once the code review pass

@firedev firedev added QA needed Needs testing by QA team code review needed Needs code review by engineer labels Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code review needed Needs code review by engineer QA needed Needs testing by QA team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants