github: vendor isGhes() func from @actions/artifact module#974
github: vendor isGhes() func from @actions/artifact module#974crazy-max merged 1 commit intodocker:mainfrom
Conversation
Signed-off-by: CrazyMax <[email protected]>
crazy-max
force-pushed
the
vendor-isghes
branch
from
0aaba9c to
386d77d
Compare
|
|
||
| // Can't use the isGhes() func from @actions/artifact due to @actions/artifact/lib/internal/shared/config | ||
| // being internal since ESM-only packages do not support internal exports. | ||
| // https://github.com/actions/toolkit/blob/8351a5d84d862813d1bb8bdeef87b215f8a946f9/packages/artifact/src/internal/shared/config.ts#L27 |
There was a problem hiding this comment.
Would it make sense to use a tag instead of a commit as reference? That way it may be easier in future to consider "this was forked a few versions ago, let's look if there are any changes since vX.Y.Z"?
There was a problem hiding this comment.
They don't use tags anymore unfortunately: https://github.com/actions/toolkit/tags
There was a problem hiding this comment.
That's ... weird!! They try to move everyone to immutable and their own stuff doesn't even use tags? 🤯
There was a problem hiding this comment.
Yes very weird, it makes it hard to link npm module version with actual git ref
| const isGitHubHost = hostname === 'GITHUB.COM'; | ||
| const isGitHubEnterpriseCloudHost = hostname.endsWith('.GHE.COM'); | ||
| const isLocalHost = hostname.endsWith('.LOCALHOST'); | ||
| return !isGitHubHost && !isGitHubEnterpriseCloudHost && !isLocalHost; |
There was a problem hiding this comment.
Any reason not to copy the code verbatim (same names etc)?
| return !isGitHubHost && !isGitHubEnterpriseCloudHost && !isLocalHost; | |
| const ghUrl = new URL( | |
| process.env['GITHUB_SERVER_URL'] || 'https://github.com' | |
| ) | |
| const hostname = ghUrl.hostname.trimEnd().toUpperCase() | |
| const isGitHubHost = hostname === 'GITHUB.COM' | |
| const isGheHost = hostname.endsWith('.GHE.COM') | |
| const isLocalHost = hostname.endsWith('.LOCALHOST') | |
| return !isGitHubHost && !isGheHost && !isLocalHost |
There was a problem hiding this comment.
isGitHubEnterpriseCloudHost is more accurate to not confuse with GitHub Enterprise alone.
There was a problem hiding this comment.
This one would still be nice though (if it works); it helps visually comparing and make sure we didn't diverge / missed changes from upstream.
There was a problem hiding this comment.
Doh! Race condition in our comments 😂
Yeah, perhaps just renaming the var could work; mostly trying to keep them as close as possible 😅
2 participants
relates to actions/toolkit#2266
needed for #972
Can't use the
isGhes()func anymore from@actions/artifactdue to@actions/artifact/lib/internal/shared/configbeing internal since ESM-only packages do not support internal exports.