Skip to content
This repository has been archived by the owner on Mar 18, 2021. It is now read-only.

Added generics to app #18

Merged
merged 7 commits into from
Nov 11, 2015
Merged

Added generics to app #18

merged 7 commits into from
Nov 11, 2015

Conversation

itsjoeconway
Copy link
Contributor

@rosshambrick @JesseBlack82 @anachlas

Made Application class generic. This means that instead of

var app = new Application();
app.pipelineType = Pipeline;

The following is now the appropriate way (once merged)

var app = new Application<Pipeline, Request>();

Where Pipeline is the app specific Pipeline, and Request is either REsourceRequest or possibly an app specific ResourceRequest. The purpose of providing ResourceRequest here is that you can now provide another class to provide options on the REsourceREquest. Right now, options are provided by adding key-value pairs to the context property of the ResourceRequest; e.g., the PostgresqlModelAdapter.

We should now be able to supply this as a property.

class WildfireRequest extends ResourceRequest {
   PostgresqlModelAdapter adapter;
}

var app = new Application<WildfirePipeline, WildfireRequest();

Then, in your pipeline, you can set these values a lot more intuitively:

Future willReceiveRequest(WildfireRequest req) {
   req.adapter = this.adapter;
}

Submitted as PR for review and to avoid conflicts on your current projects

@anachlas
Copy link
Contributor

I think we should remove the context from the ResourceRequest entirely and just create a ResourceRequest subclass that contains the adapter, permissions, and auth server for Wildfire. I never liked the idea of passing around arbitrary objects in a dictionary.

@@ -2,8 +2,11 @@
<project version="4">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should add the workspace.xml to the .gitignore

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do

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