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

Add context field to HTTPServerRequest #1550

Closed
wants to merge 1 commit into from
Closed

Conversation

thaven
Copy link
Contributor

@thaven thaven commented Aug 16, 2016

The context field allows application specific data to be passed down the chain of request processors.
Solves #1529

The context field allows application specific data to be passed down the chain of request processors.
Solves vibe-d#1529
@wilzbach
Copy link
Member

wilzbach commented Feb 6, 2017

@thaven your PR needs a rebase.

@s-ludwig what's your opinion on this? On #1529 there are many people in favor of a way to store custom information within the current request.

@s-ludwig
Copy link
Member

s-ludwig commented Feb 7, 2017

Definitely makes sense, I even remember having implemented this once, but for some reason it then got deferred. I'm just trying to remember now which name I've used and whether "context" is any better or worse.

One thing regarding the implementation: DictionaryList!(Variant, true, 4) would probably be a better idea than Variant[string], so that a few entries can be set without additional GC allocations.

@schveiguy
Copy link
Contributor

One thing regarding the implementation

Sounds good, I really look forward to this update! @s-ludwig, you can just edit the PR branch, no? It's possible @thaven isn't working on this any more as the PR is so old.

@thaven
Copy link
Contributor Author

thaven commented Feb 17, 2017

I do not have much spare time at the moment, so feel free to edit the PR.
@s-ludwig: Using DictionaryList seems a good idea to me.

@s-ludwig
Copy link
Member

@thaven: Alright, I'll change to DictionaryList and merge!

@s-ludwig s-ludwig closed this in 9d0a75b Feb 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants