Return Reponse on functions too #347

New Issue

ankitm123 · 2020-05-23T13:21:24Z

ankitm123 commented

2020-05-23 13:21:24 +00:00

The function signature of getResponse should be changed to return the response code explicitely. Presently, it returns it inside the error message, and the end user has to parse the error to find the response code. It would be user friendly to change the signature to this:

func (c *Client) getResponse(method, path string, header http.Header, body io.Reader) ([]byte, responseCode, error) {

I can work on a PR, if it is ok. For example, github and bitbucket return it separately.

this will close #303

The function signature of getResponse should be changed to return the response code explicitely. Presently, it returns it inside the error message, and the end user has to parse the error to find the response code. It would be user friendly to change the signature to this: ``` func (c *Client) getResponse(method, path string, header http.Header, body io.Reader) ([]byte, responseCode, error) { ``` I can work on a PR, if it is ok. For example, github and bitbucket return it separately. --- this will close #303

6543 commented

2020-05-23 13:51:50 +00:00

if no error ocure - the expected status is recived, I think we dont need to return the status code.

But for error responce we should, so I think this is a dublicate of #303 and refactor func getResponse() is one of the results of it ...

@ankitm123 did I missed something?

if no error ocure - the expected status is recived, I think we dont need to return the status code. But for error responce we should, so I think this is a dublicate of #303 and refactor `func getResponse()` is one of the results of it ... @ankitm123 did I missed something?

ankitm123 commented

2020-05-23 14:28:06 +00:00

if no error ocure - the expected status is recived, I think we dont need to return the status code.

In that case, should it not return data,<*response>, nil? (Better to return the pointer to the response object, and let the end user work with it?). This also brings consisteny with other git providers, wdyt?

But for error responce we should, so I think this is a dublicate of #303 and refactor func getResponse() is one of the results of it ...

@ankitm123 did I missed something?

I dont think error should contain the response/status code. A separate response struct would make the code simpler imo. So something like this:

resp, err := c.doRequest(method, path, header, body)
if err != nil {
	return nil, resp, err
}
...

thoughts?

> if no error ocure - the expected status is recived, I think we dont need to return the status code. In that case, should it not return `data,<*response>, nil`? (Better to return the pointer to the response object, and let the end user work with it?). This also brings consisteny with other git providers, wdyt? > But for error responce we should, so I think this is a dublicate of #303 and refactor `func getResponse()` is one of the results of it ... > > @ankitm123 did I missed something? I dont think error should contain the response/status code. A separate response struct would make the code simpler imo. So something like this: ``` resp, err := c.doRequest(method, path, header, body) if err != nil { return nil, resp, err } ... ``` thoughts?

6543 commented

2020-05-23 15:49:40 +00:00

I would then make it similar to https://github.com/google/go-github/blob/master/github/github.go#L405

I can work on a PR, if it is ok

@ankitm123 if you like

I would then make it similar to https://github.com/google/go-github/blob/master/github/github.go#L405 > I can work on a PR, if it is ok @ankitm123 if you like

👍 2

6543 changed title from ~~Return reponse code separately for getresponse function~~ to Return Reponse on functions

2020-05-23 15:57:35 +00:00

6543 added this to the v0.13.0 milestone 2020-05-23 15:57:39 +00:00

6543 changed title from ~~Return Reponse on functions~~ to Return Reponse on functions too

2020-05-23 15:58:55 +00:00

6543 added the

kind/feature

kind/breaking

labels 2020-05-23 15:59:03 +00:00

6543 commented

2020-06-03 22:55:30 +00:00

@ankitm123 what's the current status?

ankitm123 commented

2020-06-04 11:20:02 +00:00

I have time this week to focus on this issue, should be done by the End of Week.

👍 1 🎉 1

ankitm123 referenced a pull request that will close this issue

2020-06-07 14:22:08 +00:00

WIP feat: return response on functions #361

ankitm123 commented

2020-06-07 14:45:47 +00:00

While going through the codebase, I came across getStatusCode function, which I think can be removed after this change. Thoughts?
May be it should be done in a separate isse, and not this?

While going through the codebase, I came across `getStatusCode` function, which I think can be removed after this change. Thoughts? May be it should be done in a separate isse, and not this?

6543 commented

2020-06-07 20:55:50 +00:00

No go on with remove it

6543 referenced this issue

2020-09-05 05:19:48 +00:00

Own Error Types #303

6543 referenced a pull request that will close this issue

2020-09-12 04:32:12 +00:00

All Function return http responce #416

6543 added the

has/pull

label 2020-09-12 21:12:28 +00:00