Bug 223611

Summary: [webkitscmpy] Base GitHub mock links on provided remote
Product: WebKit Reporter: Jonathan Bedard <jbedard>
Component: Tools / TestsAssignee: Jonathan Bedard <jbedard>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, dewei_zhu, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=221860
Attachments:
Description Flags
Patch none

Description Jonathan Bedard 2021-03-22 16:16:40 PDT
Mock links in GitHub responses should be based on the remote provided.
Comment 1 Radar WebKit Bug Importer 2021-03-22 16:16:51 PDT
<rdar://problem/75713760>
Comment 2 Jonathan Bedard 2021-03-22 16:20:44 PDT
Created attachment 423958 [details]
Patch
Comment 3 Aakash Jain 2021-03-22 16:34:15 PDT
Comment on attachment 423958 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=423958&action=review

> Tools/Scripts/libraries/webkitscmpy/webkitscmpy/mocks/remote/git_hub.py:211
> +            '            <include-fragment src="/{project}/tree-commit/{ref}" aria-label="Loading latest commit">\n'

now that project contains complete url (starting from github.example.com), should the src still start with /?
Comment 4 Jonathan Bedard 2021-03-22 16:37:21 PDT
Comment on attachment 423958 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=423958&action=review

>> Tools/Scripts/libraries/webkitscmpy/webkitscmpy/mocks/remote/git_hub.py:211
>> +            '            <include-fragment src="/{project}/tree-commit/{ref}" aria-label="Loading latest commit">\n'
> 
> now that project contains complete url (starting from github.example.com), should the src still start with /?

That's why line 229 does "self.remote.split('/')[1:]", we don't want the github.example.com bit, just the team name/repository name
Comment 5 EWS 2021-03-22 17:47:28 PDT
Committed r274830: <https://commits.webkit.org/r274830>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423958 [details].