Skip to content
This repository was archived by the owner on Nov 20, 2018. It is now read-only.

[NoMerge] React to StringValues#1010

Closed
benaadams wants to merge 2 commits into
aspnet:masterfrom
benaadams:react-to-StringValues
Closed

[NoMerge] React to StringValues#1010
benaadams wants to merge 2 commits into
aspnet:masterfrom
benaadams:react-to-StringValues

Conversation

@benaadams
Copy link
Copy Markdown
Contributor

@benaadams benaadams commented Mar 21, 2018

Isn't it only HeaderDictionary that needs the change to not add nulls to the dictionary in the IDictionary path as it does in the IHeaderDictionary path?

Also needs dotnet/extensions#330

/cc @Tratcher @ryanbrandenburg @dougbu @rynowak @halter73

@benaadams
Copy link
Copy Markdown
Contributor Author

e.g. currently its saying there is a header; then returning null; whereas it should say there isn't a header

@ryanbrandenburg
Copy link
Copy Markdown
Contributor

Looks like the tests fail with this change.

@benaadams
Copy link
Copy Markdown
Contributor Author

Looks like the tests fail with this change.

Also needs dotnet/extensions#330

@benaadams
Copy link
Copy Markdown
Contributor Author

I can put a fix in (e.g. put .ToArray(); back), then revert after?

@benaadams
Copy link
Copy Markdown
Contributor Author

Added fix back

@Tratcher
Copy link
Copy Markdown
Member

Heads up, the original change was reverted. We'll reintroduce it more cautiously by doing full universe builds before merging.

@benaadams benaadams force-pushed the react-to-StringValues branch from 65ca37e to f220f6e Compare March 24, 2018 03:57
@benaadams benaadams changed the title React to StringValues [NoMerge] React to StringValues Mar 24, 2018
@natemcmaster natemcmaster changed the base branch from dev to master July 2, 2018 17:58
@muratg
Copy link
Copy Markdown

muratg commented Oct 10, 2018

We’re closing this PR because we don’t feel that this change is a good fit for the product at this time.

We thank you for the contribution and look forward to collaborating more in the future.

@muratg muratg closed this Oct 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants