RFR: 1912: Show priority for bugs in pull request body [v2]
Zhao Song
zsong at openjdk.org
Tue May 23 22:23:08 UTC 2023
On Tue, 23 May 2023 21:55:25 GMT, Erik Joelsson <erikj at openjdk.org> wrote:
>> Zhao Song has updated the pull request incrementally with seven additional commits since the last revision:
>>
>> - fix some problems
>> - fix a problem
>> - delete IssueWorkItem
>> - change initializedPR to set
>> - fix synchronization
>> - print priority for all issues except CSR and JEP
>> - introduce PRRecord
>
> bots/common/src/main/java/org/openjdk/skara/bots/common/BotUtils.java line 44:
>
>> 42: }
>> 43:
>> 44: public static Set<String> parseIssues(String body) {
>
> If this method is made public in the common module, then it needs a bit of documentation about what it does (parses issues from PR body and filters out JEP and CSR) and what it returns (list of issue IDs).
Sure. Will add some comments
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 756:
>
>> 754: }
>> 755: }
>> 756: log.info("Map after updated: " + workItem.bot.issuePRMap());
>
> Logging the whole map here is probably not a good idea in production.
Ok, I will remove it.
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/PRRecord.java line 65:
>
>> 63: return repoName + "#" + prId;
>> 64: }
>> 65: }
>
> With record I meant actually using the new record feature. It's pretty neat.
> Suggestion:
>
> public record PRRecord(String repoName, String prId);
Oh, I didn't know Java had this feature before. I will use it! Thanks!
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBot.java line 339:
>
>> 337: issuePRMap.putIfAbsent(issueId, new LinkedList<>());
>> 338: synchronized (issuePRMap.get(issueId)) {
>> 339: List<PRRecord> prRecords = issuePRMap.get(issueId);
>
> I would suggest only fetching the list once and then synchronizing on it.
Sure
-------------
PR Review Comment: https://git.openjdk.org/skara/pull/1523#discussion_r1203087858
PR Review Comment: https://git.openjdk.org/skara/pull/1523#discussion_r1203091012
PR Review Comment: https://git.openjdk.org/skara/pull/1523#discussion_r1203089495
PR Review Comment: https://git.openjdk.org/skara/pull/1523#discussion_r1203090368
More information about the skara-dev
mailing list