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

Change to Continuation format #71

Merged
merged 4 commits into from
Jul 30, 2017

Conversation

gerardtoconnor
Copy link
Member

As discussed, PR is using existing async format (not Task), and is just updating Httphandlers to double parameter format that includes continuation.

@dustinmoris dustinmoris changed the base branch from master to develop July 30, 2017 09:30
@dustinmoris dustinmoris merged commit 9151a86 into giraffe-fsharp:develop Jul 30, 2017
@dustinmoris
Copy link
Member

dustinmoris commented Jul 30, 2017

Thanks for this PR. You've put a lot of work into this project already and just want to say thanks for that! This is a great improvement. I changed the base to develop and I might think about some naming before releasing this, but I am planning to create a new release with this change in the next couple days!

I was thinking perhaps we keep calling the HttpCont a HttpHandler (what it was originally) and then type HttpPipeline = HttpHandler -> HttpHandler (or something to that effect). What do you think?

@gerardtoconnor
Copy link
Member Author

If you wanna use that name format no problem, was just trying to avoid confusion (keeping HttpHandler as the composable fn), although on the composable handler, perhaps we use 'HttpPipe' as pipeline describes line of pipes, such that we can describe a web app as a handler pipeline? (Doesn't make much difference but to me pipeline always makes me think of the entire composed pipeline).

@dustinmoris
Copy link
Member

Yeah I hear what you say... agree with you that pipeline is the entire thing put together. I will think a bit more about different names and if you have any suggestions please let me know. I just want to make sure that we pick some names that will make it easier for developers to talk about when discussing a Giraffe web application architecture, etc.

@gerardtoconnor
Copy link
Member Author

Yeah fully agree, names have to be clear so there is no confusion discussing application architecture. Would it be worth ccing a few other main users/contributors to chime in on naming convention, so we cover all bases?

@dustinmoris
Copy link
Member

sure! @slang25 @JonCanning @xdaDaveShaw @nicolocodev @Neftedollar @toburger @GraanJonlo @diegobfernandez @dsincl12

I cc'd everyone who was listed as a contributor under this project so far. Everyone welcome to chime in!

@dustinmoris
Copy link
Member

I'll throw in a few words for brainstorming: pipe, pipeline, handler, bond, circuit, chain, link, belt, flow, ...

@dustinmoris
Copy link
Member

dustinmoris commented Jul 30, 2017

@gerardtoconnor What do you think of this:

type HttpActionResult = Async<HttpContext option>
type HttpAction       = HttpContext -> HttpActionResult
type HttpHandler      = HttpAction -> HttpAction

EDIT:
One thing which I noticed is when I was searching for "HttpCont" to find all usages it brings up a lot of results because of "HttpContext". With HttpAction, HttpContext and HttpHandler the names are distinctive enough and I think its easy to reason about an architecture using these terms, but I'd be interested in your and other opinions if people care :).

@gerardtoconnor
Copy link
Member Author

Good point on the names being too similar/subset, needs to be clearly different to avoid confusion.

On the HttpAction naming, might that cause confusion with c# parameterless lambda? I think HttpAction might confuse some people?

If we are sticking with HttpHander for the composable part, we just need to figure out type name of continuation? Potential Alt names could be:

HttpNext
HttpPipe
HttpFunc

@gerardtoconnor gerardtoconnor deleted the cont-async branch July 30, 2017 22:37
@dustinmoris
Copy link
Member

Hi, I've renamed it to HttpAction yesterday (was working on it before you posted your suggestions) and I have checked that in for now, but just want to say that it doesn't mean I am settled on it yet.

I thought of HttpAction, because even though C# has an Action, they still named the MVC methods inside a Controller an "Action" as well, so I thought a HttpAction could make sense in Giraffe's context. I also like HttpFunc, but there's a Func in C# as well (Action with return type) and I thought when speaking to someone then HttpAction sounds more rounded than HttpFunc?

@gerardtoconnor
Copy link
Member Author

gerardtoconnor commented Jul 31, 2017

I was suggesting Func only due to it making more sense than Action if you are drawing a comparison to c# lambdas, as Func is a delegate that accepts parameters (`HttpContext1 here). The use of "Action" in MVC never made much sense to me as it confuses the lambda language and as action is usually a command/instruction in isolation of an input, to me it does not make sense as a name for a continuation function, but that being said, if everyone else thinks it's naturally fitting, I have no problem going with it as there are other things to work on and we shouldn't get too bogged down in naming.

How do you feel on HttpPipe ? It is nicely distinct, fits in with pipeline language, only small shortfall being that it is not the descriptively the best choice for a continuation function.

Fine with leaving as is for now if you prefer, I'd rather focus on Task CE, than worrying too much about the name.

@dustinmoris
Copy link
Member

dustinmoris commented Jul 31, 2017

I think pipe is too abstract. I can settle on HttpFunc if you're happy with it too :)

@gerardtoconnor
Copy link
Member Author

gerardtoconnor commented Aug 1, 2017 via email

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.

2 participants