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

Backlog: check if PR template was deleted #40

Open
alexellis opened this issue Mar 3, 2018 · 16 comments
Open

Backlog: check if PR template was deleted #40

alexellis opened this issue Mar 3, 2018 · 16 comments

Comments

@alexellis
Copy link
Owner

alexellis commented Mar 3, 2018

Scenario: impatient contributor deletes whole PR template message and decides it doesn't apply to them.

Result: PR is decorated with a comment telling user to fill out PR template.

Implementation ideas:

Opt in via feature in .DEREK.yml file pr_template_checking

  • v1.0 PR body cannot be empty
  • v2.0 100% of the lines beginning with a heading i.e. # in repo/.github/PULL_REQUEST_TEMPLATE.md must be present in body of PR

@rgee0 @dirkhh @johnharris85 thoughts?

bad_pr_edi

@rgee0
Copy link
Contributor

rgee0 commented Mar 3, 2018

I’d be tempted to also have a switch to enable totally empty PRs to be automatically closed. Default would be just apply a message.

@johnharris85
Copy link

Ye I like the idea (and feature progression). Are you thinking same thing for issues as well?

Also what about the ability to check that a PR references an existing issue / proposal? That ties into the workflow for OpenFaaS that all PRs must have issues / proposals.

@alexellis
Copy link
Owner Author

@johnharris85 the same thing could apply to issues but for an MVP I'd suggest going after PRs first. I guess this would be valuable to any project that uses a template for PRs.

The issue per PR requirement is a thing, but I think it may be hard to automate so could be revisited later.

If anyone wants to work on a solution as per v1.0 or v2.0 please comment

@johnmccabe
Copy link
Contributor

I was starting to have a look at this, if nobody else is already working on it.

@johnmccabe
Copy link
Contributor

I've got this working, just need to take some time to rebase and raise the PR, will try to get it raised by wednesday

@dirkhh
Copy link
Contributor

dirkhh commented Apr 16, 2018

BTW, I missed this earlier... I think v2.0 is a bit too aggressive. It's fairly common for PR templates to have sections that don't really make sense for all PRs... So I'd say that "at least half of the headings need to remain" or something slightly more flexible than that. Extra credits for it being configurable :-)

@alexellis
Copy link
Owner Author

Bump on this issue and +1 to Dirk's comments I think that makes sense.. Some proportion of headings should be present. Certainly it will pick up the annoying fly-by PRs like:

(no text given)

Alex

@alexellis
Copy link
Owner Author

@johnharris85 would you like to take a look at this one? I also have a refactoring task that needs doing to make Derek more like an SDK / easier to run in other platforms/apps. #62 (I don't have a good way to contact you so a comment here is the best at the moment, if you'd like to join OpenFaaS Slack we have a #derek channel there)

@alexellis
Copy link
Owner Author

It seems like we have a kind of dead-lock or lack of interest for this feature. I still have to comment back to people who delete the whole text and submit a PR with no description at all. For that reason I've raised a simpler issue for v1.0 of the feature in #103

@burtonr
Copy link
Contributor

burtonr commented Sep 25, 2019

Getting started on this now. Hoping for it to be ready for the HacktoberFest.

Please let me know if any other features or anything has come to mind since the last time this was commented on

@alexellis
Copy link
Owner Author

Did anyone work on this?

@alexellis
Copy link
Owner Author

alexellis commented Jan 2, 2020

Going back to the "Implementation ideas:", I'd suggest that we make a binary decision based upon the following criteria for “version 1.5” of this

  • A PR template is present in the repo
  • 50% of the lines beginning with # in the template are still in the PR text submitted

@alexellis
Copy link
Owner Author

A new contributor can start this with a unit test in isolation from Derek's main codebase and PR/webhook handling features, once fully working we can help with integrating it in.

@tchaudhry91
Copy link

I'm getting started on this now

@burtonr
Copy link
Contributor

burtonr commented Jan 2, 2020

Version "v1.0" was completed here: #104

This change/update would be for version "v2.0" where the template is used to validate the incoming PR body.

@alexellis
Copy link
Owner Author

Aha, yes so I think we need version “1.5” and not quite a “2.0” as the original idea there might have been a bit too strict.

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

7 participants