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

Print debug information #4

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Print debug information #4

wants to merge 6 commits into from

Conversation

nilscox
Copy link

@nilscox nilscox commented Apr 24, 2023

No description provided.

@nilscox nilscox force-pushed the print-debug-info branch 2 times, most recently from 42cac0a to cc06a96 Compare April 24, 2023 09:59
@brmzkw
Copy link
Collaborator

brmzkw commented Apr 24, 2023

I believe the following changes are necessary, even if they might be out of the scope of this pull request:

  • we should add -i to curl to get the headers
  • we can split the "Install Koyeb CLI" step into two steps:
  1. fetch the latest release and output it (echo "$releases" ... | grep | awk | sed ... >> "$GITHUB_OUTPUT") see GITHUB_OUTPUT in the github doc
  2. mkdir and download the file

Also, it would be great to allow the action to be run more than once. Currently, the second run will fail because mkdir attempts the $bin_dir directory which already exists. We could test if koyeb already exists and output a message "koyeb-cli already installed, skip installation"

@nilscox nilscox force-pushed the print-debug-info branch 2 times, most recently from fa916be to 3de7f6d Compare April 24, 2023 10:28
@nilscox
Copy link
Author

nilscox commented Apr 24, 2023

we should add -i to curl to get the headers

Good idea.

we can split the "Install Koyeb CLI" step into two steps:

I agree, makes things clearer.

Also, it would be great to allow the action to be run more than once.

I added a step to check this before everything else, let me know if you would have done it differently.

@brmzkw
Copy link
Collaborator

brmzkw commented Apr 24, 2023

I didn't try to run the action, but it looks good! Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants