-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Build status should be on target repo #21
base: master
Are you sure you want to change the base?
Conversation
Having played with the 1.6.0 version a bit, I found that I had to change the repository_owner and repository_name name from head_reponame and head_owner to repository_name and owner, respectively. Originally, I got the status reported on the source repository, on the previous commit in my repo. What I wanted was to have the status reported on the PR on the target repo. I admit I haven't done a huge amount of reading of the code, but what it looks like to me is that head_reponame and head_owner are refering to the source repo, so I changed those to repository_name and owner, which - at least for my setup - point to the target repository of the PR. Furthermore, in the 1.6.0 code, sha was set to sourcestamp['revision'], which for me points to the previous commit in the repo, meaning the status would be reported on the wrong commit in case of a PR. So for PRs (which I think all have 'head_sha' in props), I am setting sha to props['head_sha']. The result of this change for me is that the status of PR testing is reported on the target repo on the correct commit (so showing up as green checkmarks / yellow balls / red crosses on the PR). For normal commits on this same repo, this also works.
I realize you have probably tested this on your own set up, so I figure we'll probably have to do some more digging and find a way to make this work for PRs within the same repo and PRs between repos. |
The head sha could be correct, the other changes would be identical to the old behaviour, see literally 2 lines down from your changes. |
Mmh. Looking at b4d11a7 I see what you mean. Bit painful. I was really excited about the 1.6.0 release, and when it didn't work for me, tried to make it work. As said, I didn't dive into the code really deep. Probably should have. However, with 1.6.0 I did get status updates in the wrong place. Looking at my PR in BB, head_repository points at the source repo, and I assume that head_reponame goes with that. But I admittedly still struggle with BuildBots terminology... |
Those are basically terminology from gitea/git. Gitea sends the json for a pull request from a fork, see below. This wants to pull from It might be using the wrong SHA (have not tested the SHA thoroughly), but the repo should be correct, given the assumptions above. {
"secret": "test",
"action": "opened",
"number": 4,
"pull_request": {
"id": 36,
"url": "https://git.example.com/testuser/webhook_test/pulls/4",
"number": 4,
"user": {
"id": 1,
"login": "testuser",
"full_name": "testuser name",
"email": "[email protected]",
"avatar_url": "https://git.example.com/user/avatar/testuser/-1",
"language": "en-US",
"is_admin": true,
"last_login": "2021-03-27T13:53:28Z",
"created": "2018-06-05T09:41:06Z",
"username": "testuser"
},
"title": "testfork",
"body": "Test PR",
"labels": [],
"milestone": null,
"assignee": null,
"assignees": null,
"state": "open",
"is_locked": false,
"comments": 0,
"html_url": "https://git.example.com/testuser/webhook_test/pulls/4",
"diff_url": "https://git.example.com/testuser/webhook_test/pulls/4.diff",
"patch_url": "https://git.example.com/testuser/webhook_test/pulls/4.patch",
"mergeable": true,
"merged": false,
"merged_at": null,
"merge_commit_sha": null,
"merged_by": null,
"base": {
"label": "master",
"ref": "master",
"sha": "449a5a8ca05607106b5ba41988c1a658a8949a18",
"repo_id": 20,
"repo": {
"id": 20,
"owner": {
"id": 1,
"login": "testuser",
"full_name": "testuser name",
"email": "[email protected]",
"avatar_url": "https://git.example.com/user/avatar/testuser/-1",
"language": "en-US",
"is_admin": true,
"last_login": "2021-03-27T13:53:28Z",
"created": "2018-06-05T09:41:06Z",
"username": "testuser"
},
"name": "webhook_test",
"full_name": "testuser/webhook_test",
"description": "",
"empty": false,
"private": true,
"fork": false,
"template": false,
"parent": null,
"mirror": false,
"size": 76,
"html_url": "https://git.example.com/testuser/webhook_test",
"ssh_url": "ssh://[email protected]/testuser/webhook_test.git",
"clone_url": "https://git.example.com/testuser/webhook_test.git",
"original_url": "",
"website": "",
"stars_count": 0,
"forks_count": 1,
"watchers_count": 1,
"open_issues_count": 0,
"open_pr_counter": 1,
"release_counter": 0,
"default_branch": "master",
"archived": false,
"created_at": "2018-09-04T10:45:23Z",
"updated_at": "2018-09-04T13:05:51Z",
"permissions": {
"admin": false,
"push": false,
"pull": false
},
"has_issues": true,
"internal_tracker": {
"enable_time_tracker": false,
"allow_only_contributors_to_track_time": true,
"enable_issue_dependencies": true
},
"has_wiki": true,
"has_pull_requests": true,
"has_projects": false,
"ignore_whitespace_conflicts": false,
"allow_merge_commits": true,
"allow_rebase": true,
"allow_rebase_explicit": true,
"allow_squash_merge": true,
"avatar_url": "",
"internal": false
}
},
"head": {
"label": "feature_branch",
"ref": "feature_branch",
"sha": "53e3075cbe468f14c2801d186d703e64b2adee12",
"repo_id": 34,
"repo": {
"id": 34,
"owner": {
"id": 14,
"login": "test_org",
"full_name": "",
"email": "",
"avatar_url": "https://git.example.com/user/avatar/test_org/-1",
"language": "",
"is_admin": false,
"last_login": "1970-01-01T00:00:00Z",
"created": "2018-09-27T11:35:41Z",
"username": "test_org"
},
"name": "webhook_test_fork",
"full_name": "test_org/webhook_test_fork",
"description": "",
"empty": false,
"private": true,
"fork": true,
"template": false,
"parent": {
"id": 20,
"owner": {
"id": 1,
"login": "testuser",
"full_name": "testuser name",
"email": "[email protected]",
"avatar_url": "https://git.example.com/user/avatar/testuser/-1",
"language": "en-US",
"is_admin": true,
"last_login": "2021-03-27T13:53:28Z",
"created": "2018-06-05T09:41:06Z",
"username": "testuser"
},
"name": "webhook_test",
"full_name": "testuser/webhook_test",
"description": "",
"empty": false,
"private": true,
"fork": false,
"template": false,
"parent": null,
"mirror": false,
"size": 76,
"html_url": "https://git.example.com/testuser/webhook_test",
"ssh_url": "ssh://[email protected]/testuser/webhook_test.git",
"clone_url": "https://git.example.com/testuser/webhook_test.git",
"original_url": "",
"website": "",
"stars_count": 0,
"forks_count": 1,
"watchers_count": 1,
"open_issues_count": 0,
"open_pr_counter": 2,
"release_counter": 0,
"default_branch": "master",
"archived": false,
"created_at": "2018-09-04T10:45:23Z",
"updated_at": "2018-09-04T13:05:51Z",
"permissions": {
"admin": false,
"push": false,
"pull": false
},
"has_issues": true,
"internal_tracker": {
"enable_time_tracker": false,
"allow_only_contributors_to_track_time": true,
"enable_issue_dependencies": true
},
"has_wiki": true,
"has_pull_requests": true,
"has_projects": false,
"ignore_whitespace_conflicts": false,
"allow_merge_commits": true,
"allow_rebase": true,
"allow_rebase_explicit": true,
"allow_squash_merge": true,
"avatar_url": "",
"internal": false
},
"mirror": false,
"size": 19,
"html_url": "https://git.example.com/test_org/webhook_test_fork",
"ssh_url": "ssh://[email protected]/test_org/webhook_test_fork.git",
"clone_url": "https://git.example.com/test_org/webhook_test_fork.git",
"original_url": "",
"website": "",
"stars_count": 0,
"forks_count": 0,
"watchers_count": 1,
"open_issues_count": 0,
"open_pr_counter": 0,
"release_counter": 0,
"default_branch": "master",
"archived": false,
"created_at": "2021-03-28T21:40:46Z",
"updated_at": "2021-03-28T21:41:01Z",
"permissions": {
"admin": false,
"push": false,
"pull": false
},
"has_issues": true,
"internal_tracker": {
"enable_time_tracker": false,
"allow_only_contributors_to_track_time": true,
"enable_issue_dependencies": true
},
"has_wiki": true,
"has_pull_requests": true,
"has_projects": true,
"ignore_whitespace_conflicts": false,
"allow_merge_commits": true,
"allow_rebase": true,
"allow_rebase_explicit": true,
"allow_squash_merge": true,
"avatar_url": "",
"internal": false
}
},
"merge_base": "449a5a8ca05607106b5ba41988c1a658a8949a18",
"due_date": null,
"created_at": "2021-03-28T21:41:24Z",
"updated_at": "2021-03-28T21:41:24Z",
"closed_at": null
},
"repository": {
"id": 20,
"owner": {
"id": 1,
"login": "testuser",
"full_name": "testuser name",
"email": "[email protected]",
"avatar_url": "https://git.example.com/user/avatar/testuser/-1",
"language": "en-US",
"is_admin": true,
"last_login": "2021-03-27T13:53:28Z",
"created": "2018-06-05T09:41:06Z",
"username": "testuser"
},
"name": "webhook_test",
"full_name": "testuser/webhook_test",
"description": "",
"empty": false,
"private": true,
"fork": false,
"template": false,
"parent": null,
"mirror": false,
"size": 76,
"html_url": "https://git.example.com/testuser/webhook_test",
"ssh_url": "ssh://[email protected]/testuser/webhook_test.git",
"clone_url": "https://git.example.com/testuser/webhook_test.git",
"original_url": "",
"website": "",
"stars_count": 0,
"forks_count": 1,
"watchers_count": 1,
"open_issues_count": 0,
"open_pr_counter": 2,
"release_counter": 0,
"default_branch": "master",
"archived": false,
"created_at": "2018-09-04T10:45:23Z",
"updated_at": "2018-09-04T13:05:51Z",
"permissions": {
"admin": true,
"push": true,
"pull": true
},
"has_issues": true,
"internal_tracker": {
"enable_time_tracker": false,
"allow_only_contributors_to_track_time": true,
"enable_issue_dependencies": true
},
"has_wiki": true,
"has_pull_requests": true,
"has_projects": false,
"ignore_whitespace_conflicts": false,
"allow_merge_commits": true,
"allow_rebase": true,
"allow_rebase_explicit": true,
"allow_squash_merge": true,
"avatar_url": "",
"internal": false
},
"sender": {
"id": 1,
"login": "testuser",
"full_name": "testuser name",
"email": "[email protected]",
"avatar_url": "https://git.example.com/user/avatar/testuser/-1",
"language": "en-US",
"is_admin": true,
"last_login": "2021-03-27T13:53:28Z",
"created": "2018-06-05T09:41:06Z",
"username": "testuser"
},
"review": null
} |
Ok, I think I made the head_sha change last, so I'll check and see if with reverting the second change it still works tomorrow. Will report back after. Thanks @pampersrocker |
Ok I tested, because why test tomorrow if you can test today. Sadly, it needs both changes to work for me. I'm with you for most part of what you write above, but if the status is reported against head/repo, then the test result from buildbot will show up on the source repo (34). That might make sense in some cases, I guess, but I would expect the results to be reported against the target repo (20), don't you think? Isn't that what GitHub does? Or am I misreading the json? (Also I was confused about the concept of a sourcestamp in buildbot, but let's let that slide for now ;) ) |
From my limited testing, I need to set the status of the Setting the Setting the Setting the The 1.6.0 changes basically from Case 2 to Case 4 in a PR scenario, but ideally we need Case 1, which is your SHA change of this pull request. |
Pleas note. i've committed the head SHA change to the master and made Version v1.7.0 out of it, but had no time to investigate the fork situation further. |
Having played with the 1.6.0 version a bit, I found that I had to change the repository_owner and repository_name name from head_reponame and head_owner to repository_name and owner, respectively.
Originally, I got the status reported on the source repository, on the previous commit in my repo. What I wanted was to have the status reported on the PR on the target repo.
I admit I haven't done a huge amount of reading of the code, but what it looks like to me is that head_reponame and head_owner are refering to the source repo, so I changed those to repository_name and owner, which - at least for my setup - point to the target repository of the PR.
Furthermore, in the 1.6.0 code, sha was set to
sourcestamp['revision']
, which for me points to the previous commit in the repo, meaning the status would be reported on the wrong commit in case of a PR.So for PRs (which I think all have 'head_sha' in props), I am setting sha to
props['head_sha']
.The result of this change for me is that the status of PR testing is reported on the target repo on the correct commit (so showing up as green checkmarks / yellow balls / red crosses on the PR).
For normal commits on this same repo, this also works.