Closed maennchen closed 7 months ago
Thanks for this. I'll try to find out why the Actions didn't run, but the change seems sane.
Would you mind rebasing as a way to force-push something and check what happens with CI? Thanks.
@paulo-ferraz-oliveira It seems like you don't have on: pr: ...
rules in your workflows. It therefore only triggers for pushed onto the original repository.
You need something like this: https://github.com/erlef/oidcc/blob/366f725b71f9552be5e1d77cc1a108d67237f5d4/.github/workflows/pr.yml#L2
That makes sense.
Or just on: [push, workflow_dispatch, pull_request]
in the header.
Careful: If you enable push
for every branch and pull_request
as well, it will run twice if you do a PR from this repo and not a fork.
I usually do this:
on:
push:
branches:
- main
pull_request:
branches:
- "*"
workflow_dispatch: {}
If you enable push for every branch and pull_request as well, it will run twice if you do a PR from this repo and not a fork.
That's what we had before and reverted because we thought only push
would do the job.
I've merged the pull_request
stuff to the main branch.
@paulo-ferraz-oliveira Rebased and it seems to work :+1:
Would you add a simple test? Maybe just take what's there, and extend it to have a file under /tmp
and then pass the absolute path.
@paulo-ferraz-oliveira I've changed it so that i tests both relative and absolute paths.
There's no need to keep rebase-squashing the changes 😄 (it forces the reviewer to read the commit all over again).
If required (though it's already our policy before merging) we'd kindly ask you to squash everything together.
I'll look at what I have now, though.
Just two minor comments (not worth tweaking what doesn't need tweaking - since it changes history and makes for harder future spelunking) but Ok otherwise.
@paulo-ferraz-oliveira I though it makes sense to group the statements together so that it is easier to understand.
I removed the extra lines.
I though it makes sense to group the statements together so that it is easier to understand.
Yeah, but during a review it only makes it harder:
You keep force-pushing even though I've given you reasons and politely asked you not to. 😄
Your changes will be squash-merged before going to the main branch, via GitHub.
@paulo-ferraz-oliveira Oh, ups, I missed that comment. I'll stop doing that.
It's Ok. I'll merge now and check if we're Ok to release. Thanks.
Released in v1.17.4 (v1 and v1.17 reset to the same commit).
@paulo-ferraz-oliveira Thanks! The tags v1
and v1.17
still refer to a23b1fc though.
Yeah, I didn't force push 😄 Thanks for noticing.
Done.
Description
When using an absolute path like
/tmp/.tool-versions
, the action would look in the file/$GITHUB_WORKSPACE/tmp/.tool-versions
. This change lets it consider absolute paths as well.