Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

idea #2

Draft
wants to merge 35 commits into
base: main
Choose a base branch
from
Draft

idea #2

wants to merge 35 commits into from

Conversation

jcbhmr
Copy link

@jcbhmr jcbhmr commented Jan 30, 2024

DO NOT MERGE. expect this pr to be edited a bunch.

Hello! 👋

I'm the guy who saw this action like 6 months ago and thought "hey that doesn't quite fit what I want to do; let me make my own version". Now I'd like to re-merge some of the best parts of that adventure back into this project. It's the de-facto setup-cloudflared action out there and I figured I might as well try to help improve that rather than trying to make my own version.

This is a "hey look at this; is this a good direction?" pr and to make you aware of my fork and that id like to contribute.

changes so far:

  • use "start" and "stop" terminology to be similar to npm start, npm stop, systemctl start, systemctl stop.
  • use sub-actions instead of overloaded options for the start operation
  • in start: use run: key similar to how - run: <cmd> works normally; now it's just nested a bit.
  • try to improve the readme; pictures, maybe some emoji, idk
  • still support sign in credentials as part of - uses: .../setup-cloudfared's with inputs
  • supports semver ranges in the cloudflared-version input
  • uses js instead of composite for the root setup-cloudflared action
  • use bash/powershell for everything
  • github action to auto create a v1 tag for any v1.x.y release

ref #1

end goal:

- uses: AnimMouse/setup-cloudfared@v2
- run: npm install --global serve
- uses: AnimMouse/setup-cloudfared/start@v2
  with:
    url: http://localhost:8000
    run: serve --port 8000
- run: sleep 10m

notes:

@jcbhmr jcbhmr marked this pull request as draft January 30, 2024 21:02
@AnimMouse
Copy link
Owner

Hello, I really liked your suggestion, I created this action because back in the day, we don't have a way to test programs in CI, but when GitHub Codespaces got released, I moved all the live testing away from Actions and into Codespaces, this is why I haven't updated this action since then since I'm currently not using this action. Since knowing that there are still people like you are using this action, I think I will update this if I got time.

About the move to JavaScript based action, I think that will never happen in the near future. I'm a self-proclaimed expert in Bash and PowerShell. I really wanted to create an action back then, but when composite based action are now possible, I successfully used my scripting skills to create an action like this. Furthermore, I have only written JavaScript in the client side, not in the server side, so if we moved to JavaScript, I think I cannot maintain this action anymore.

I recommend you to just release a JavaScript-based setup-cloudflared action that you maintain and everyone can use. For now, I will try implementing your suggestions in Bash and PowerShell way.

@jcbhmr
Copy link
Author

jcbhmr commented Feb 4, 2024

About the move to JavaScript based action, I think that will never happen in the near future. I'm a self-proclaimed expert in Bash and PowerShell. I really AnimMouse/setup-rclone#7, but when composite based action are now possible, I successfully used my scripting skills to create an action like this. Furthermore, I have only written JavaScript in the client side, not in the server side, so if we moved to JavaScript, I think I cannot maintain this action anymore.

I re-did everything using Bash. The only part that I got tripped up by is the "find the most recent version that satisfies this semver range: ^2023.6.6". I solved this by bundling a semver.mjs compiled artifact (from https://npm.im/semver) cli tool that does do semver stuffs from the cli. I don't know if there's a better way to do this. Feel free to tweak/edit things if you have any better ideas/tweaks or anything.

Let me know if this Bash-only style action is acceptable

@jcbhmr
Copy link
Author

jcbhmr commented Feb 4, 2024

upon further googling i think that this https://github.com/qzb/sh-semver is an option? maybe. problem is that it has some weird errors (? i think they're errors) and is also quite slow compared to semver.mjs

jcbhmr@PIG-2016:~/Documents/semversh$ time bash semver.sh -r "^2023.0.0" $(gh release list --repo cloudflare/cloudflared
)
semver.sh: line 138: [: Latest: integer expression expected
semver.sh: line 142: [: Latest: integer expression expected
semver.sh: line 138: [: Latest: integer expression expected
semver.sh: line 142: [: Latest: integer expression expected
semver.sh: line 138: [: Latest: integer expression expected
semver.sh: line 142: [: Latest: integer expression expected
semver.sh: line 138: [: Latest: integer expression expected
semver.sh: line 142: [: Latest: integer expression expected
semver.sh: line 138: [: Latest: integer expression expected
semver.sh: line 142: [: Latest: integer expression expected
semver.sh: line 138: [: Latest: integer expression expected
semver.sh: line 142: [: Latest: integer expression expected
2023.1.0
2023.1.0
2023.2.1
2023.2.1
2023.2.2
2023.2.2
2023.3.0
2023.3.0
2023.3.1
2023.3.1
2023.4.0
2023.4.0
2023.4.1
2023.4.1
2023.4.2
2023.4.2
2023.5.0
2023.5.0
2023.5.1
2023.5.1
2023.6.0
2023.6.0
2023.6.1
2023.6.1
2023.7.0
2023.7.0
2023.7.1
2023.7.1
2023.7.2
2023.7.2
2023.7.3
2023.7.3
2023.8.0
2023.8.0
2023.8.1
2023.8.1
2023.8.2
2023.8.2
2023.10.0
2023.10.0

real    0m17.655s
user    0m15.114s
sys     0m4.962s

@AnimMouse AnimMouse added the enhancement New feature or request label Feb 4, 2024
@jcbhmr jcbhmr marked this pull request as ready for review February 21, 2024 17:01
@jcbhmr jcbhmr marked this pull request as draft February 21, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants