RFR: 1912: Show priority for bugs in pull request body [v2]

Erik Joelsson erikj at openjdk.org
Tue May 23 21:08:56 UTC 2023


On Tue, 23 May 2023 17:08:41 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java line 324:
>> 
>>> 322: 
>>> 323:     private void initializeIssuePRMap() {
>>> 324:         // When bot restarts, the issuePRMap needs to get updated with this pr
>> 
>> I'm not sure about this comment. This mapping needs to be updated every time there is a change to the associations, or if the map is currently missing associations for this PR. I don't think it makes sense to maintain an `initializedPRs` map, we should just always update the map.
>> 
>> This update process needs to be synchronized. The map is a `ConcurrentHashMap`, which is good, but we need to synchronize on the list value before trying to manipulate it.
>
> I make the bot maintaining the` initializedPRs` map because I think there is no need to always update the map here. We  only need to update the map here when the bot restarts and it's the first time for the bot to see this pr. Otherwise, the update is extra work. 
> 
> The real update for the map located in `CheckRun#getStatusMessage` from line 744 to 768. 
> 
> And yes, I need to synchronize on the list value before trying to manipulate it. Thanks for catching!

Ah, I missed that part, that's good. With multiple locations doing updates to the map, I would recommend adding methods `addIssuePrMapping` and `removeIssuePrMapping` to `PullRequestBot` so you can encapsulate the dealing with lists and synchronization. Probably also `getIssuePrMapping`.

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

PR Review Comment: https://git.openjdk.org/skara/pull/1523#discussion_r1202771907


More information about the skara-dev mailing list