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

Make the query script simpler and more robust #3

Merged
merged 1 commit into from
Jan 30, 2022

Conversation

generalmimon
Copy link
Contributor

@generalmimon generalmimon commented Jan 24, 2022

Make the query script simpler and more robust

@generalmimon
Copy link
Contributor Author

My motive behind this pull request is that sometime between 2021-07-01 and 2021-11-06, GitHub started to add ?check_suite_focus=true query parameter to URLs, which made gha-jobid-action output "4126623327?check_suite_focus=true" as job_id. Originally I just wanted to open an issue about it, but then decided to take a stab at fixing it myself, so did just that and also simplified and robustized the command at the same time.

@generalmimon generalmimon force-pushed the simplify-and-make-robust branch 3 times, most recently from 585cbfc to fd10912 Compare January 26, 2022 11:29
@Tiryoh
Copy link
Owner

Tiryoh commented Jan 27, 2022

@generalmimon Thanks for the PR! I'll check it out this weekend.

@Tiryoh Tiryoh self-assigned this Jan 27, 2022
@Tiryoh Tiryoh self-requested a review January 27, 2022 21:43
@Tiryoh Tiryoh added Type: Bug Bug or Bug fixes Type: Refactoring A code change that neither fixes a bug nor adds a feature labels Jan 27, 2022
@Tiryoh Tiryoh linked an issue Jan 27, 2022 that may be closed by this pull request
* Use `--get` instead of `-X GET`, as recommended in `curl` docs
  ( https://curl.se/docs/manpage.html#-X,
  https://curl.se/docs/manpage.html#-G )
* Set the HTTP header `Accept: application/vnd.github.v3+json`, as
  recommended throughout the GitHub REST API docs:
  https://docs.github.com/en/rest/reference/actions#list-jobs-for-a-workflow-run
* Make the actual command simpler and more robust (avoids potential
  quoting issues, does not attempt to parse JSON output or the URL with
  `grep` or `sed` which is inherently unreliable and error-prone, etc.),
  using the approach from
  https://github.com/hashicorp/terraform-provider-external/blob/v2.2.0/website/docs/data_source.html.md#processing-json-in-shell-scripts
* Double-quote all variables according to https://www.shellcheck.net/,
  see https://github.com/koalaman/shellcheck/wiki/SC2086
* Add newline at end of file
@Tiryoh
Copy link
Owner

Tiryoh commented Jan 30, 2022

It's awesome!
After adding test, I'll merge this.

@Tiryoh Tiryoh merged commit 01224f4 into Tiryoh:master Jan 30, 2022
@Tiryoh
Copy link
Owner

Tiryoh commented Jan 30, 2022

Thanks again for your contribution, @generalmimon.
I have added your credit on README (#7).

@generalmimon generalmimon deleted the simplify-and-make-robust branch January 30, 2022 09:05
@Tiryoh Tiryoh mentioned this pull request Nov 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Bug or Bug fixes Type: Refactoring A code change that neither fixes a bug nor adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

job_id contains "?check_suite_focus=true"
2 participants