search

jasminsehic / infinity.net

.NET API for Azure DevOps and DevOps Server formerly Visual Studio Online and Team Foundation Server
Other
36 stars 13 forks source link

Adding JSON headers to GetItem requests. Fixes #8 #9

Closed andrewcoll closed 9 years ago

andrewcoll commented 9 years ago

Adding accept JSON headers to ensure GetItem and GetItems returns metadata instead of file contents.

ethomson commented 9 years ago

Cool, first pull request! Two minor nits, if you don't mind:

  1. For the sake of consistency, could we use Request.AddHeader(string, string) instead of Request.Headers.Add(...)? (eg, like https://github.com/awec/infinity.net/blob/master/Infinity/Clients/GitClient.cs#L748 )
  2. It looks like you've pushed a merge, and so there's three commits landing in the PR instead of one. Would you mind rebasing so that it's just a single commit?

Thanks!

ethomson commented 9 years ago

So... also... I'm relatively sure (but I can't trust my memory 100%) that in the 1.0 preview API that this behaved differently.

Do you think that we should add Accept: application/json to all requests by default (unless they're overridden)? (Not saying that you necessarily need to do this work, I'm just curious if you have thoughts here).

andrewcoll commented 9 years ago

I'd say it would do no harm to add it to all requests actually.

I had a quick look through the docs and this appears to be the only scenario that calls out that the presence of the header will affect what is actually returned, but as the majority of requests return JSON anyway I guess it should be there by default.

I'll can make the changes you mentioned in your first comment too.

ethomson commented 9 years ago

I guess it should be there by default.

I think that the download scenarios switch on content-type (application/zip is a requirement, I believe). I feel like I remember something else requiring application/octet-stream but I may be misremembering at this point.

In any case, this is probably outside the scope of this PR.

ethomson commented 9 years ago

Thanks!