RFR: 1254: Add a checkbox for CSR requirement

Erik Joelsson erikj at openjdk.java.net
Mon Nov 29 15:28:11 UTC 2021

On Sat, 27 Nov 2021 13:54:51 GMT, Guoxiong Li <gli at openjdk.org> wrote:

> Hi all,
> This patch adds a checkbox for CSR requirement in the `Progress` and adds the corresponding test cases.
> Thanks for taking the time to review.
> Best Regards,
> -- Guoxiong

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 176:

> 174:             ret.put("Change requires a CSR request to be approved", false);
> 175:         } else {
> 176:             ret.put("Change doesn't require a CSR request or the CSR request has been approved", true);

I don't think we should show anything here if a CSR was never requested. The difference between "CSR was never requested" and "CSR is approved" isn't really tracked currently, so that would be tricky. I'm guessing this is a big reason why the original implementation didn't add checkboxes, but only a blocker message when active. For this to work we will need to add some state that tracks if a CSR is currently required. We basically have 3 states:

1. No CSR needed
2. CSR needed and not yet approved
3. CSR needed and approved

Every PR starts in state 1. To get to state 2, you can either issue the /csr command, or with SKARA-1071 file a CSR and let Skara find it. To get to state 3, the CSR must be approved. At any time, the command "/csr unneeded" can take you back to state 1.

This could be implemented with another label, or it could rely on special comments with markers. In this case, I think we want to make it visible to users, so labels probably make more sense. For backwards compatibility, to keep the current meaning of the "csr" label, the new label would need to be something like "csr-approved" and be added at the same time the csr label is removed when moving to state 3.


PR: https://git.openjdk.java.net/skara/pull/1246

More information about the skara-dev mailing list