Test-infra: k8s-ci-robot suggests PR author as the additional approver

[porridge]

See https://github.com/kubernetes/contrib/pull/2825#issuecomment-356655365
There are other OWNERs than myself.

[stevekuznetsov]

/area prow
/kind bug

[stevekuznetsov]

/cc @kargakis @cjwagner

[cjwagner]

@porridge
This is actually WAI even though it looks strange. In the contrib repo, implicit self-approval is disabled (by default): https://github.com/kubernetes/test-infra/blob/f7f8aec6acd0f005053f0185379ccce6f72710bd/prow/plugins.yaml#L17-L34

You hadn't actually approved the PR yourself yet and your approval would have helped approve the PR so you were suggested. In repos where the self-approval is disabled, PR authors need to /approve their own PR if they want to approve it themselves.

[porridge]

Do you know the history of implicit_self_approve and why is it not on for
this particular repo?

[cjwagner]

I kept the same settings when migrating the repo from using the approval-handler munger to the approve Prow plugin.
I'm happy to make kubernetes/contrib more consistent with other k8s repos though. I think that it almost always makes sense to have implicit_self_approve enabled.
@kubernetes/contrib-maintainers

[stevekuznetsov]

The impetus for being able to toggle implicit self-approval mostly came before /hold but in general follows this logic:

• as a top-level OWNER, I create a PR and want it to pass tests that may or may not be part of the default presubmits
• I want to have reviews from N people
• someone unrelated comes by and /lgtm my PR, and it merges before it's ready because I implicitly self-approved it

[cjwagner]

Something else to note is that if implicit self-approval is enabled, a PR author can still remove their own approval with /approve cancel and add it back if later if they want to.

[porridge]

FWIW, an explicit /approve cancel or /hold sound much better to me than the
current (somewhat surprising) default.