bugflow / auto-gnome

GitHub Ticket Gnome
https://auto-gnome.readthedocs.org/
0 stars 1 forks source link

create PR grooming plugin #14

Open monkeypants opened 6 years ago

monkeypants commented 6 years ago
As a project tech lead
I need a PR grooming plugin
so that I can ensure my desired process is followed

I imagine .gnome.yml might look something like:

plugins:
 - PRChecklist:
   - code_review: Code has been reviewed
   - test_coverage: New/Changed code has good test coverage
   - builds_ok: This code builds OK in my local environment

And as a result:

Given I have a repo gnome
And I have PRChecklist plugin enabled
And at least one checklist item is defined
When someone makes a PR
Then the gnome makes a PR Checklist comment on the PR

PR CHecklist comment might look something like this:


PR Checklist:

Also, another result of the plugin:

Given I have a repo gnome
And I have PRChecklist plugin enabled
And at least one checklist item is defined
When someone merges a PR
And the checklist is not all checked
Then the gnome raises a ticket to query the merge
And the raised ticket is assigned to the person who merged the PR

The raised ticket might like something like this


When PR #1234567 was merged, the following PR Checklist items were not checked:

Was this an accident? Please re-check and close this ticket after making any necessary rectification.

If this checklist item was deliberately omitted, please explain why it wasn't appropriate for this PR (then close the ticket).


monkeypants commented 6 years ago

hey @ltankey, I raised this ticket in response to our conversation this morning. Did I miss anything, is does that just about capture the requirement?

ltankey commented 6 years ago

I think that captures it!

I'm not sure about the additional ticket - it might be annoying, but then it shouldn't be happening every time. Perhaps there's an override option, or perhaps we just keep it simple and accept that the user will have to close the raised ticket.

monkeypants commented 6 years ago
plugins:
 - PRChecklist:
   - items:
     - code_review: Code has been reviewed
     - test_coverage: New/Changed code has good test coverage
     - builds_ok: This code builds OK in my local environment
  - raise_issues: True

???

ltankey commented 6 years ago

Yes or even potentially a comment_on_original_ticket, with something like

A pull request was closed for this issue, but it doesn't look like code review checklist was completed.

It then provides the QA stream with more information

monkeypants commented 6 years ago

if PR with no ticket reference, ask for one?

ltankey commented 6 years ago

Yeah in this context, they shouldn't be doing a PR without a ticket, so that would be a check something should do. Might be a task for this plugin, particularly if it is responding to an opened PR

monkeypants commented 6 years ago

what about commits without ticket reference?

ltankey commented 6 years ago

I think that's a style thing. I've never minded commits that don't have references if they're part of a PR that does, as it chews up precious message space. We could have a nag option but it feels like more of a chatops feature

monkeypants commented 6 years ago

precious message space on git commit comments? they can be as big as you like (practically).

first line -50 characters is good practie blank line as much stuff as you can be bothered writing...

monkeypants commented 6 years ago

but nvm; the question is "should PR Checklist plugin have an opinion about linking tickets to PRs?" Maybe that should be a different policy.

ltankey commented 6 years ago

I've never liked those other lines, but I take the point.

In any case, whilst it isn't something I'd use it doesn't mean someone else wouldn't

monkeypants commented 6 years ago

let's build it if/when we need it :)

ltankey commented 6 years ago

Yeah you're probably right. It could be a "ensure referential integrity of PRs" policy

monkeypants commented 6 years ago

Maybe we should rename "plugins" to "opinions".

monkeypants commented 6 years ago

And make naming convention that says what the opinion is.

ltankey commented 6 years ago

Sounds like a good way of having things named by what they do, which is good

ltankey commented 6 years ago

Very declarative :)

monkeypants commented 6 years ago

.gnome.yml

opinions:
 - PR_checklist_should_be_checked:
   - items:
      - code_review: Code has been reviewed
      - test_coverage: New/Changed code has good test coverage
      - builds_ok: This code builds OK in my local environment
  - raise_sooky_tickets: True
 - PR_should_reference_issue
 - verbose_logging_is_good
 - use_a_sorting_hat
monkeypants commented 6 years ago

I'm a but worried this ticket is reproducing stuff that might be done better with existing jenkins plugins.