RFR: 1408: GitLabMergeRequest#reviews sometimes finds the wrong commit HASH [v2]

Guoxiong Li gli at openjdk.java.net
Mon May 2 16:26:16 UTC 2022


> Hi all,
> 
> Please consider the following steps:
> 
> - the author creates "commit 1" locally (with hash "95f2d613cf392275075d7c87285845531f5fe827" [1])
> - the author pushes it and creates a merge request in the Gitlab, then the Gitlab creates "version 1" [2] (its hash is the hash of "commit 1") which has the changed files of "commit 1"
> - the author creates "commit 2" locally (with hash "99ca51edfa0063113a2236fce548ba453aee7275" [3])
> - a reviewer approves the MR with the "**version 1**" in the Gitlab. Note: the "commit 2" is not pushed now.
> - the author creates "commit 3" locally (with hash "50b119c4c325053b8151ea6e761a60d1acfbb746" [4]) Note: this step seems optional
> - the author pushes the commits and the gitlab create "version 2" [5] (its hash is the hash of "commit 3") which has the changed files of "commit 1-3"
> 
> When we use the method `GitLabMergeRequest#reviews` to get the reviews list, we can see the review hash is the hash of the "commit 2" ("99ca51edfa0063113a2236fce548ba453aee7275"). But actually, the reviewer only approved "version 1" with hash of the "commit 1" ("95f2d613cf392275075d7c87285845531f5fe827").
> 
> This patch uses the `versions` list instead of all the `commits` list to fix the issue. And when I wrote a test case to verify it, I got a NPE and filed SKARA-1409 to record it. This patch also solves SKARA-1409 so that the reviewer can run the test locally.
> 
> You can use the following config to run the test:
> 
> gitlab.user=<your username in https://gitlab.com>
> gitlab.pat=<your token in https://gitlab.com>
> 
> gitlab.uri=https://gitlab.com
> gitlab.repository=35596381
> gitlab.merge.request.id=2
> gitlab.review.hash=95f2d613cf392275075d7c87285845531f5fe827
> 
> 
> Note: this PR seems have git conflict with [SKARA-1407-PATCH](https://github.com/openjdk/skara/pull/1304). I will solve the conflict when one of them is integrated.
> 
> Thanks for taking the time to review.
> 
> Best Regards,
> -- Guoxiong
> 
> [1] https://gitlab.com/lgxbslgx/test/-/merge_requests/2/diffs?commit_id=95f2d613cf392275075d7c87285845531f5fe827
> [2] https://gitlab.com/lgxbslgx/test/-/merge_requests/2/diffs?diff_id=379883693
> [3] https://gitlab.com/lgxbslgx/test/-/merge_requests/2/diffs?commit_id=99ca51edfa0063113a2236fce548ba453aee7275
> [4] https://gitlab.com/lgxbslgx/test/-/merge_requests/2/diffs?commit_id=50b119c4c325053b8151ea6e761a60d1acfbb746
> [5] https://gitlab.com/lgxbslgx/test/-/merge_requests/2/diffs?diff_id=379907777
> 
> ---

Guoxiong Li has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits:

 - Merge branch 'master' into SKARA-1408
   
   # Conflicts:
   #	forge/src/test/java/org/openjdk/skara/forge/gitlab/GitLabRestApiTest.java
 - Disable test.
 - 1409: The GitLab 'list-users' api doesn't return 'email' field for normal users
 - 1408: GitLabMergeRequest#reviews sometimes finds the wrong commit HASH

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

Changes: https://git.openjdk.java.net/skara/pull/1306/files
 Webrev: https://webrevs.openjdk.java.net/?repo=skara&pr=1306&range=01
  Stats: 20 lines in 3 files changed: 16 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/skara/pull/1306.diff
  Fetch: git fetch https://git.openjdk.java.net/skara pull/1306/head:pull/1306

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


More information about the skara-dev mailing list