Integrated: forge: fix GitLabRepository.recentCommitComments

Erik Helin ehelin at openjdk.java.net
Wed Feb 24 13:02:50 UTC 2021


On Tue, 23 Feb 2021 15:01:29 GMT, Erik Helin <ehelin at openjdk.org> wrote:

> Hi all,
> 
> please review this patch that re-works `GitLab.recentCommitComments` into its hopefully final shape. This patch fixes two problems:
> 
> - multiple commits can have the same commit message title (i.e. first line of commit message)
> - GitLab sometimes returns a trimmed `target_title` for commit comment events
> 
> The first problem is fixed by always starting out with fetching the entire history (once) and then always return a set of candidate hashes for a given commit title. We can then fetch the commit comments for each of the candidate hashes to figure which hash the comment was made on. The solution to the second problem then piggy-backs on the first solution: if a hash with a given title isn't found, and the title ends with `...`, then go through all commit message titles and check if any of them has the given title as prefix.
> 
> The drawback is that it can take a couple of minutes to populate the initial commit message title to hash mapping, but since 874a89ceb0d5fd742db75b6f9744b56e200cfe32 this will run in a separate `WorkItem`, so the degraded performance won't affect other `WorkItem`s during startup. Once the map is populated there will only be one additional REST request for most calls to `recentCommitComments`.
> 
> Testing:
> - [x] `make test` passes on Linux x64
> - [x] Manual testing against GitLab on Linux x64 with `git skara debug commit-comments`
> 
> Thanks,
> Erik

This pull request has now been integrated.

Changeset: 12a8c32a
Author:    Erik Helin <ehelin at openjdk.org>
URL:       https://git.openjdk.java.net/skara/commit/12a8c32a
Stats:     135 lines in 2 files changed: 80 ins; 44 del; 11 mod

forge: fix GitLabRepository.recentCommitComments

Reviewed-by: rwestberg

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

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


More information about the skara-dev mailing list