Skip to content

github: return output if JSON parsing is not desired#7252

Merged
MikeMcQuaid merged 1 commit intoHomebrew:masterfrom
dawidd6:github-not-json
Apr 1, 2020
Merged

github: return output if JSON parsing is not desired#7252
MikeMcQuaid merged 1 commit intoHomebrew:masterfrom
dawidd6:github-not-json

Conversation

@dawidd6
Copy link
Copy Markdown
Contributor

@dawidd6 dawidd6 commented Apr 1, 2020

There are endpoints that do not return a JSON object, like for example:
https://developer.github.com/v3/actions/workflow_runs/#list-workflow-run-logs

Add an option to not parse the response data as JSON.

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?

Comment thread Library/Homebrew/utils/github.rb Outdated
Comment thread Library/Homebrew/utils/github.rb Outdated
@dawidd6 dawidd6 force-pushed the github-not-json branch 2 times, most recently from b7e1660 to 96b059e Compare April 1, 2020 10:12
@dawidd6 dawidd6 changed the title github: return output if not_json github: return output if JSON parsing is not desired Apr 1, 2020
@dawidd6 dawidd6 force-pushed the github-not-json branch 2 times, most recently from a80cc37 to c2adfac Compare April 1, 2020 10:18
Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even simpler version suggested

Comment thread Library/Homebrew/utils/github.rb Outdated
Comment thread Library/Homebrew/utils/github.rb Outdated
Comment thread Library/Homebrew/utils/github.rb Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
parse_json ? json : output
parse_json output

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You meant output only right?

@MikeMcQuaid
Copy link
Copy Markdown
Member

Looks great, thanks @dawidd6!

@MikeMcQuaid MikeMcQuaid merged commit 1cbf261 into Homebrew:master Apr 1, 2020
@dawidd6 dawidd6 deleted the github-not-json branch April 1, 2020 11:46
@lock lock bot added the outdated PR was locked due to age label May 5, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants