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

Version 3.0.0 #9

Merged
merged 4 commits into from Jan 11, 2017
Merged

Version 3.0.0 #9

merged 4 commits into from Jan 11, 2017

Conversation

rowanmanning
Copy link
Member

@rowanmanning rowanmanning commented Jan 10, 2017

This work forms the basis of v3.0.0 switches to a middleware-based API. Aside from the rename from express-ftwebservice to @financial-times/express-web-service, the key differences are illustrated here:

const express = require('express');
const expressWebService = require('@financial-times/express-web-service');
const app = express();
const options = {};

// Old
expressWebService(app, options);

// New
app.use(expressWebService(options));

This partly resolves #5 – we now create standard Express middleware. You can't access the request object in the about endpoint, but it can now be generated asynchronously.

The best way to view any breaking changes is to read the migration guide.

@rowanmanning rowanmanning changed the title [WIP] Version 3.0.0 Version 3.0.0 Jan 11, 2017
@rowanmanning rowanmanning merged commit 116999b into master Jan 11, 2017
@rowanmanning rowanmanning deleted the version-3 branch January 11, 2017 14:38
Copy link
Member

@lucas42 lucas42 left a comment

Choose a reason for hiding this comment

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

This looks much nicer to use than the old interface 😄

- `about`: (Optional) Object containing standard runbook properties as defined in the [ft about endpoint standard], or a Promise that resolves to an object containing these properties
- `goodToGoTest`: (Optional) A function that can be used to indicate the good to go status of the service, the function should return a Promise resolved with `true` to indicate a positive good to go status, and `false` to indicate a negative good to go status
- `healthCheck`: (Optional) A function that can be used to generate structured healthcheck information, the function should return a Promise resolved with an array of healthcheck objects
- `routes`: (Optional) An array of routes to install. Possible values are `health`, `gtg`, `about` and `error`. Defaults to `["health", "gtg", "about"]`
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this is immediately clear. I had to read the code before I got what it means. Perhaps an example would help?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, will add an issue for this

- `about`: (Optional) Object containing standard runbook properties as defined in the [ft about endpoint standard], or a Promise that resolves to an object containing these properties
- `goodToGoTest`: (Optional) A function that can be used to indicate the good to go status of the service, the function should return a Promise resolved with `true` to indicate a positive good to go status, and `false` to indicate a negative good to go status
- `healthCheck`: (Optional) A function that can be used to generate structured healthcheck information, the function should return a Promise resolved with an array of healthcheck objects
- `routes`: (Optional) An array of routes to install. Possible values are `health`, `gtg`, `about` and `error`. Defaults to `["health", "gtg", "about"]`
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this is immediately clear. I had to read the code before I got what it means. Perhaps an example would help?

};

// Serve the __gtg endpoint
// TODO
Copy link
Member

Choose a reason for hiding this comment

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

What's TODO here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, will remove

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.

Limitations of approach unusual for express middleware
3 participants