search

screwdriver-cd / screwdriver

An open source build platform designed for continuous delivery.
http://screwdriver.cd
Other
1.02k stars 170 forks source link

Draft pull requests #2102

Open wahapo opened 4 years ago

wahapo commented 4 years ago

What happened: This is a trivial consultation.

Regarding the correspondence of Draft pull requests, I would like to clarify what kind of policy is the best in Screwdriver.cd.

Currently, PR build will start even if PR is draft. It may be correct for the PR build to start building after the review is ready.

What you expected to happen: I'm not sure which is right.

How to reproduce it:

  1. Create pipeline
  2. Create draft PR
tkyi commented 4 years ago

For me personally, I'm confused when someone might use a Draft PR. Is this the equivalent of putting WIP or DNM in the title? Is it a better practice to use Draft PR instead of putting WIP or DNM in the title?

If both practices are still acceptable in the future, we can have the PR build only start after the review is ready for Draft PR. If we should only use Draft PR in the future, I'm not sure.

okuryu commented 4 years ago

I'm using draft pull requests as a work in progress, so I always expect the behavior to run the build job.

The only difference between draft pull requests and opened pull requests is that they are ready to be reviewed and merged. It's common practice to check whether a build will be successful before doing a review.

tanoda commented 4 years ago

I agree with @okuryu.

But, I think we want to consider that draft pull-request will send notification or not. Pull-request submitter should notice build errors (but not supported in Screwdriver yet), but I think project members should not be for draft pull-request.

okuryu commented 4 years ago

ha, I forgot about the notifications because I had never turned on the notifications for all ~pr jobs. This may be related to #1274.

dwmarshall commented 4 years ago

For @tkyi, when I see a DNM pull request, I point out to the author that a draft pull request is a better mechanism.

I usually want my draft pull requests to trigger a ~pr build, but that may just indicate that I need to use sd-local more!

One suggestion: create a new event ~draft that may be handled or not.

okuryu commented 4 years ago

I'm not positive about ~draft for now as the design is likely to be complicated from the following points.

  1. GitHub draft pull request and opened pull request can be converted to each other after they are created.
    1. This conversion does not trigger a push event, so it probably won't be delivered to the Screwdriver webhooks.
    2. This feature is available on GitHub.com, but GitHub Enterprise Server requires v2.21.
  2. This mechanism is not yet standard in SCMs and is quite GitHub only.
    1. GitLab seems to have a similar feature called "Work In Progress" merge requests, but it seems to just disable the "Merge" button.
    2. Bitbucket doesn't seem to have a similar feature, but it seems to be a status under consideration.
jithine commented 4 years ago

But, I think we want to consider that draft pull-request will send notification or not. Pull-request submitter should notice build errors (but not supported in Screwdriver yet), but I think project members should not be for draft pull-request.

I would favor this approach. Let builds run like how it's today, but notification should go only to the PR author.

master-nevi commented 2 years ago

Another approach would be to surface some sort of GITHUB_IS_DRAFT environment variable and allow the job owners to decide what to do if the PR is a draft.