Skip to content

Conversation

@robertbrignull
Copy link
Contributor

Requires some changes on the server side before this will work / can be merged.

This sends the action ref, and the tools input and resolved version as part of the status reports. Getting the resolved tools version in a nice format proved slightly awkward, but hopefully the result in this PR isn't too bad.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.

@robertbrignull robertbrignull force-pushed the robertbrignull/tools_version branch from 65df982 to 80b43ca Compare November 12, 2020 12:28
@robertbrignull
Copy link
Contributor Author

The failed checkrun is just some alert churn because I changed src/codeql.ts too close to an alert.

I checked the data that's coming through and the action_ref was set to v2 which I found odd. Especially as in the workflows on this repo we run the action from a local copy, so it's unclear what the ref would be. Turns out the ref value is coming from actions/checkout@v2 which was the previous step to run in the workflow. I've added a check to work around this in this PR and I'll make sure it's working. I've also opened actions/runner#803 because I think this behaviour is a bug in the actions runner.

@robertbrignull
Copy link
Contributor Author

Can confirm the action_ref is no longer being sent when running as a local action (there's probably a better name for this than "local")

Copy link
Contributor

@chrisgavin chrisgavin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I was a bit worried it might not be compatible with 2.22, but we don't send status reports on Enterprise so it seems like it should be fine. 👍

path.isAbsolute(relativeScriptPath)
) {

if (isRunningLocalAction()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for factoring this out. 👍

@robertbrignull robertbrignull merged commit dc80b01 into main Nov 18, 2020
@robertbrignull robertbrignull deleted the robertbrignull/tools_version branch November 18, 2020 11:38
@github-actions github-actions bot mentioned this pull request Nov 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants