Conversation
| const token = args[0] | ||
| const options = {...args[1]} // Shallow clone - don't mutate the object provided by the caller | ||
|
|
||
| // Base URL - GHES or Dotcom |
There was a problem hiding this comment.
I'll look before merging whether any unit test can be added. Might be able to verify the expected baseUrl is set and whether proxy is set based on the correct URL
There was a problem hiding this comment.
unfortunately doesnt appear to be a way to get the options after it's constructed
ericsciple
force-pushed
the
users/ericsciple/m261ghes
branch
from
a7b6f2a to
888957c
Compare
|
notes from offline review:
|
thboop
left a comment
thboop
left a comment
There was a problem hiding this comment.
LGTM, would be nice if we can add unit tests
| process.env['GITHUB_GRAPHQL_URL'] || 'https://api.github.com/graphql' | ||
|
|
||
| // Remove trailing "/graphql" | ||
| if (url.toUpperCase().endsWith('/GRAPHQL')) { |
There was a problem hiding this comment.
There's no possibility of a trailing slash is there? Alternative https://stackoverflow.com/a/16750711/775184
There was a problem hiding this comment.
shouldnt be a trailing slash, i'll trim on launch side when setting the urls
There was a problem hiding this comment.
also not bad to add resiliency here, i added code to trim trailing slash if detected
ericsciple
force-pushed
the
users/ericsciple/m261ghes
branch
from
98c8419 to
f872dae
Compare
ericsciple
force-pushed
the
users/ericsciple/m261ghes
branch
from
f872dae to
0c667a8
Compare
|
Going ahead and merging since failure due to audit service being down. Retried twice, still failed. Passed in a previous run and not changing dependencies with this PR. |
3 participants
Related to ADR for setting GHE URLs (in c2c-actions)
The toolkit should use the GITHUB_API_URL and GITHUB_GRAPHQL_URL to connect to GHES or Dotcom.