feat: selfapprove flag for approving policies

[lee2sh]

what

introducing SelfApprove flag to control if the author can approve.

why

I think it makes sense to have the ability to control if the author can approve the policy checks.

tests

references

• the PR author can run approve_policies. #4813

[lee2sh]

@chenrui333 any chance that this can get more 👀 ? It would be much appreciated.

[jamengual]

LGTM

[GMartinez-Sisti]

Can you update the documentation for this feature? Also is there a change on the current behavior? If so, might be considered a breaking change.

[lee2sh]

Can you update the documentation for this feature? Also is there a change on the current behavior? If so, might be considered a breaking change.

Will update the documentation. There is a slight change on the behavior where the policies would default to false if the selfapprove flag is not set, meaning the PR author would not be able to approve policies even if they are on the list of approvers.

[jlee-nydig]

@chenrui333 @jamengual @GMartinez-Sisti updated the doc. Let me know what you guys think. Also, would it be possible for this to go out on the next release?

[jamengual]

Can you update the documentation for this feature? Also is there a change on the current behavior? If so, might be considered a breaking change.

Will update the documentation. There is a slight change on the behavior where the policies would default to false if the selfapprove flag is not set, meaning the PR author would not be able to approve policies even if they are on the list of approvers.

this is a breaking change then, I don't think that Is a good option since the approved user (that happens to be the author ) can't approved it and it was added previously to the list of approvers.

[lee2sh]

Can you update the documentation for this feature? Also is there a change on the current behavior? If so, might be considered a breaking change.

Will update the documentation. There is a slight change on the behavior where the policies would default to false if the selfapprove flag is not set, meaning the PR author would not be able to approve policies even if they are on the list of approvers.

this is a breaking change then, I don't think that Is a good option since the approved user (that happens to be the author ) can't approved it and it was added previously to the list of approvers.

@jamengual So there is an option to self approve even if you are the author. But IMO, I feel like it's a best practice not to give the author the ability to approve their own PR, especially when it involves policies/guardrails.

However, this option doesn't mean that the Owners cannot approve other policy checks on different PRs. That is totally feasible.

I think the freedom to choose whether or not you can approve your own PR is good to have.

[lee2sh]

@jamengual @GMartinez-Sisti @chenrui333 could we do something like this? Now, it's not a breaking change any more. I've inverted the flag so prevent_self_approve defaults to false and the current behavior persists. We now have the option to toggle prevent_self_approve to true to block PR author from approving.

[lee2sh]

@jamengual @GMartinez-Sisti @chenrui333 Thank you for approving this ❤️. Any chance that this can go onto the next release?

[jamengual]

@jamengual @GMartinez-Sisti @chenrui333 Thank you for approving this ❤️. Any chance that this can go onto the next release?

yes, we will add it to the next release

[synack-badamson]

PreventSelfApprove bool         `yaml:"self_approve,omitempty" json:"prevent_self_approve,omitempty"`

Shouldn't both yaml and json defs here have prevent_self_approve?