Add action output for using the error message #65
Conversation
|
The remaining CI check just wants you to run This is because GitHub Actions currently must be written in JavaScript, so we have to check in the |
src/main.ts
Outdated
| stdout: (data: Buffer) => { | ||
| SEMVER_CHECKS_OUTPUT += data.toString(); | ||
| }, | ||
| stderr: (data: Buffer) => { | ||
| SEMVER_CHECKS_OUTPUT += data.toString(); | ||
| }, |
There was a problem hiding this comment.
Consider checking if data is non-empty and not undefined here. The undefined prefix we see in the output right now is probably from an invocation with data = undefined here.
| permissions: | ||
| pull-requests: write |
There was a problem hiding this comment.
Oh interesting! Does this work correctly even if the PR comes from a fork of the repo? 👀
I would have assumed this might give the job "PR write" permissions on the fork, but admittedly I find the GitHub Actions permissions model confusing and unpredictable even after working with it for years.
There was a problem hiding this comment.
Yeah I find it confusing as well. I need to find some time to set up a repo to test this - running out of time now.
There was a problem hiding this comment.
If you get a chance to test this, I'd love to make sure it works well before we put it in the docs.
There was a problem hiding this comment.
Nope, it doesn't work from a fork.
Error: Resource not accessible by integration
Any idea?
There was a problem hiding this comment.
I never got a chance to dig too deep into this, but last time I looked, one possible answer seemed to be something like "create a GitHub App." That seemed a bit annoying to do, so I didn't do it at the time.
If you find or hear about any other possible workarounds, I'd love to know about them.
There was a problem hiding this comment.
Ah, I see. Well, I'm not sure then. Creating a GitHub app definitely sounds annoying :D
| permissions: | ||
| pull-requests: write |
There was a problem hiding this comment.
If you get a chance to test this, I'd love to make sure it works well before we put it in the docs.
| const output = await runCargoSemverChecks(cargo); | ||
| core.setOutput("logs", stripAnsi(output)); |
There was a problem hiding this comment.
I just realized something. By making output available in the logs output value, are we accidentally preventing it from being printed into the GitHub workflow logs themselves?
That might be shooting ourselves in the foot, because we might prevent maintainers from seeing the output if they need to dig in for any reason. It'd be awesome to have the ability to post run output as a comment in the PR, but I don't think we want to break other users that use the action differently (e.g. before cargo publish like cargo-semver-checks uses it itself).
There was a problem hiding this comment.
I just realized something. By making output available in the logs output value, are we accidentally preventing it from being printed into the GitHub workflow logs themselves?
What do you mean? In my tests I can see the logs in both GitHub Action run and in the comment.
| permissions: | ||
| pull-requests: write |
There was a problem hiding this comment.
I never got a chance to dig too deep into this, but last time I looked, one possible answer seemed to be something like "create a GitHub App." That seemed a bit annoying to do, so I didn't do it at the time.
If you find or hear about any other possible workarounds, I'd love to know about them.
See #64
Implemented based on amannn/action-semantic-pull-request@880a3c0
Not tested, it is very straightforward so I believe this will work just fine (i.e. I can test after merge if you're okay with that :D)
Let me know what you think! 🐻