search

trapd00r / LS_COLORS

A collection of LS_COLORS definitions; needs your contribution!
Other
2.11k stars 260 forks source link

Fix github action logic #185

Closed rpdelaney closed 2 years ago

rpdelaney commented 2 years ago

We're getting build failures from the new GitHub actions. See #184

cc @ahmedelgabri

rpdelaney commented 2 years ago

Might be that it needs this, but I also have a similar workflow working without the token so I'm a bit confused.

It's already set by GitHub automatically https://docs.github.com/en/actions/security-guides/automatic-token-authentication#using-the-github_token-in-a-workflow

Could you try adding this to this section

https://github.com/trapd00r/LS_COLORS/blob/572b294639fc2823cc2c06ef18cdd92d3546774e/.github/workflows/build.yml#L18-L25

env:
  GITHUB_TOKEN: '${{ secrets.GITHUB_TOKEN }}'

I might try testing this in a fork to avoid disruption downstream. I have a lot on my plate right now but hopefully I can get to it soon. The first task will be to reproduce the failure in a fork.

ahmedelgabri commented 2 years ago

Another solution is to avoid all this logic to detect a merged PR, since there is no real official way to do it. And instead, run the action on push but only on master which will be triggered also when a PR is merged to master

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 8b4405e..f0f410d 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -1,12 +1,10 @@
 name: 'Release'
 on:
-  pull_request:
+  push:
     branches: [master]
-    types: [closed]

 jobs:
   release:
-    if: github.event.pull_request.merged == true
     runs-on: ubuntu-latest
     steps:
       - name: 'Check out Git repository'
rpdelaney commented 2 years ago

@ahmedelgabri I've been very busy with trying to close on a house. Would you be willing to send a pull request with the fix? I just have not been able to make time. :(

ahmedelgabri commented 2 years ago

@rpdelaney can you have a look at #186?

rpdelaney commented 2 years ago

Did that work?

Edit: seems like lscolors.sh wasn't rebuilt

ahmedelgabri commented 2 years ago

@rpdelaney that's because the only change that happened to the source code was #183 which generated a correct .csh file. The .sh files didn't change, so only .csh was rebuilt.

https://github.com/trapd00r/LS_COLORS/commits/master

rpdelaney commented 2 years ago

Makes sense. Looks good now: 5608c1097625f555bf4cdcb578cdf36be0980798

Thanks again!