RFR: 1532: CSRBot is too inefficient [v2]

[Erik Joelsson]

On Thu, 18 Aug 2022 13:39:42 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> bots/csr/src/main/java/org/openjdk/skara/bots/csr/PullRequestWorkItem.java line 48:
>> 
>>> 46:     private final static String CSR_LABEL = "csr";
>>> 47:     private final static String CSR_UPDATE_MARKER = "<!-- csr: 'update' -->";
>>> 48:     private static final String PROGRESS_MARKER = "<!-- Anything below this marker will be automatically updated, please do not edit manually! -->";
>> 
>> It's a bit unfortunate to duplicate string literals, especially those used as markers. Can you import this from `CheckRun.progressMarker` instead?
>
> I completely agree that duplicated strings are bad. Importing directly from another bot module wouldn't be great IMO. I will look into moving this into some common class in a shared module instead. Note though that this isn't new in this patch.

Actually, thinking on this some more, can I file a followup to attack constants shared between bots in a more uniform way? I think many of these should be shared knowledge between bots, including label names, so adding a PullRequestContants class in the forge module (next to PullRequestUtils) would be a good idea I think. I don't want to pollute this change with this though.

-------------

PR: https://git.openjdk.org/skara/pull/1357