Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Append the content headers so the header Content-Type is included in the headers list #2170

Merged
merged 1 commit into from
Nov 10, 2018
Merged

Conversation

TangoMikeOscar
Copy link

Fixes #2124

I'm not an expert with f# but it seems a simple fix and I have tested this in isolation and it fixes the bug mentioned in the other issue.

Hopefully this is ok.

@matthid
Copy link
Member

matthid commented Oct 26, 2018

Could we add a test for this?

@matthid
Copy link
Member

matthid commented Oct 26, 2018

Thanks for looking into this, yes this probably is the correct thing to do. Still a bit unexpected that the behavior changed oO

@TangoMikeOscar
Copy link
Author

Unfortunately writing tests is a bit beyond my f# capabilities, wouldn't know where to start, only really learnt enough to get Fake working with our projects. Hopefully that is not a problem.

@matthid
Copy link
Member

matthid commented Oct 29, 2018

Ok I'll try to come up with a test, but it will take a couple of days

@matthid matthid added help wanted needs-tests This PR needs tests in order to be accepted labels Oct 29, 2018
@matthid matthid self-assigned this Oct 29, 2018
@TangoMikeOscar
Copy link
Author

Sounds good thanks. We would very much like this fix so if there's anything I can help with please let me know.

@matthid
Copy link
Member

matthid commented Oct 30, 2018

Yes, I just want to ensure we don't regress on this before merging

@matthid matthid merged commit a874dc1 into fsprojects:release/next Nov 10, 2018
@matthid
Copy link
Member

matthid commented Nov 10, 2018

Thanks again for figuring out the root cause and providing a fix, I could reproduce with a test and refactored the code a bit (feel free to review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted needs-tests This PR needs tests in order to be accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NuGet.Version.getLastNuGetVersion fails to parse response
2 participants