Skip to content

feat: add NewJsonResponseOrPanic function to simplify calls with ResponderFromMultipleResponses#151

Merged
maxatome merged 1 commit into
jarcoal:v1from
kilianpaquier:dev/kpaquier/jsonresponse-panic
Apr 18, 2024
Merged

feat: add NewJsonResponseOrPanic function to simplify calls with ResponderFromMultipleResponses#151
maxatome merged 1 commit into
jarcoal:v1from
kilianpaquier:dev/kpaquier/jsonresponse-panic

Conversation

@kilianpaquier

@kilianpaquier kilianpaquier commented Apr 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Simplify calls to ResponderFromMultipleResponses for JSON body responses with NewJsonResponseOrPanic.

Just as the same as NewJsonResponderOrPanic and NewJsonResponder, it's an addition to simplify some calls and avoid local variables with error handling.

maxatome
maxatome requested changes Apr 14, 2024
View reviewed changes

@maxatome maxatome left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for your PR. See my comments to go further.

Just as a note did you know that you can chain Responder using Then method? It appeared after ResponderFromMultipleResponses implementation. Using it you can do:

responder := httpmock.NewJsonResponderOrPanic(200, x).
  Then(httpmock.NewJsonResponderOrPanic(200, y)).
  Then(httpmock.NewJsonResponderOrPanic(200, z))

Comment thread response.go Outdated
Comment thread response.go Outdated
Comment thread response.go Outdated
Comment thread response.go Outdated
Comment thread response.go Outdated
@maxatome

maxatome commented Apr 15, 2024

Copy link
Copy Markdown
Collaborator

Could you please also change the commit message to:

feat: add NewJsonResponseOrPanic function

@kilianpaquier

kilianpaquier commented Apr 16, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Thanks for your PR. See my comments to go further.

Just as a note did you know that you can chain Responder using Then method? It appeared after ResponderFromMultipleResponses implementation. Using it you can do:

responder := httpmock.NewJsonResponderOrPanic(200, x).

  Then(httpmock.NewJsonResponderOrPanic(200, y)).

  Then(httpmock.NewJsonResponderOrPanic(200, z))

Hi, yes I did see this builder function. We've been using it for the last two days, it's great too !

But I still think that NewJsonResponseOrPanic could be a useful addition, what do you think ?

@kilianpaquier

kilianpaquier commented Apr 16, 2024

Copy link
Copy Markdown
Contributor Author

Could you please also change the commit message to:


feat: add NewJsonResponseOrPanic function

Will do !

maxatome
maxatome reviewed Apr 17, 2024
View reviewed changes
Comment thread response_test.go Outdated
Comment thread response_test.go Outdated
Comment thread response.go Outdated
@maxatome maxatome merged commit d95b1ca into jarcoal:v1 Apr 18, 2024
@kilianpaquier kilianpaquier deleted the dev/kpaquier/jsonresponse-panic branch April 18, 2024 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maxatome maxatome maxatome approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kilianpaquier @maxatome