search

coder / hat

HTTP API testing for Go
MIT License
36 stars 3 forks source link

cleanup #15

Closed nhooyr closed 6 years ago

nhooyr commented 6 years ago

Closes #6

I made some API consistency changes, made RequestOption not take a T, it shouldn't ever need the T, and I also moved DuplicateBody back to Response. It feels really wrong to have it on T, that was my bad for suggesting it in the previous PR.

ammario commented 6 years ago

It would be unnatural since this is test code and inconsistent with ResponseAssertion. We use require all the time to invert control flow in tests.

On Mon, Apr 9, 2018, 12:47 PM Anmol Sethi notifications@github.com wrote:

@nhooyr commented on this pull request.

In request.go https://github.com/codercom/hat/pull/15#discussion_r180175943:

"github.com/stretchr/testify/require" )

// RequestOption modifies a request. -type RequestOption func(t T, req http.Request) +type RequestOption func(req http.Request)

How about it returns an error instead of taking a T?

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub https://github.com/codercom/hat/pull/15#discussion_r180175943, or mute the thread https://github.com/notifications/unsubscribe-auth/AHEpUDGoqQT93y3Wd4H2vOkorEdLqhoiks5tm564gaJpZM4TLvGP .

-- Thanks, Ammar Bandukwala