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

Integrate new path-to-regexp with trailing optionals [appveyor azuredevops npm gitlab date githubmanifest githubpackage jenkins matrix readthedocs requires shippable snyk wordpress] #2644

Merged
merged 15 commits into from
Jan 8, 2019
Merged
Prev Previous commit
Next Next commit
Merge branch 'master' into path-to-regexp-update-2
  • Loading branch information
paulmelnikow committed Jan 8, 2019
commit 6c11e6e4c6c21d83ae9582c47f1f270a7127bcb7
112 changes: 53 additions & 59 deletions doc/rewriting-services.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,19 @@ module.exports = class ExampleService extends LegacyService {
References:

- Current documentation
- [Defining a route][]
- [Defining examples][]
- [Creating a tester][]
- [BaseService, the new service framework][baseservice]
- [LegacyService, the adapter][legacyservice]
- [Defining a route][]
- [Defining examples][]
- [Creating a tester][]
- [BaseService, the new service framework][BaseService]
- [LegacyService, the adapter][LegacyService]
- [Obsolete tutorial on legacy services][old tutorial], possibly useful as a reference

[old tutorial]: https://github.com/badges/shields/blob/e25e748a03d4cbb50c60b69d2b2404fc08e7cead/doc/TUTORIAL.md
[defining a route]: https://github.com/badges/shields/blob/master/doc/TUTORIAL.md#42-our-first-badge
[defining examples]: https://github.com/badges/shields/blob/master/doc/TUTORIAL.md#44-adding-an-example-to-the-front-page
[creating a tester]: https://github.com/badges/shields/blob/master/doc/service-tests.md#1-boilerplate
[baseservice]: https://github.com/badges/shields/blob/master/services/base.js
[legacyservice]: https://github.com/badges/shields/blob/master/services/legacy-service.js
[Old tutorial]: https://github.com/badges/shields/blob/e25e748a03d4cbb50c60b69d2b2404fc08e7cead/doc/TUTORIAL.md
[Defining a route]: https://github.com/badges/shields/blob/master/doc/TUTORIAL.md#42-our-first-badge
[Defining examples]: https://github.com/badges/shields/blob/master/doc/TUTORIAL.md#44-adding-an-example-to-the-front-page
[Creating a tester]: https://github.com/badges/shields/blob/master/doc/service-tests.md#1-boilerplate
[BaseService]: https://github.com/badges/shields/blob/master/services/base.js
[LegacyService]: https://github.com/badges/shields/blob/master/services/legacy-service.js

## First, write some tests

Expand All @@ -64,26 +64,26 @@ tests are passing, though.
## Organization

1. When there’s a single legacy service that handles lots of different things
(e.g. version, license, and downloads), it should be split into three separate
service classes and placed in three separate files, e.g.:
(e.g. version, license, and downloads), it should be split into three separate
service classes and placed in three separate files, e.g.:

- `example-version.service.js`
- `example-license.service.js`
- `example-downloads.service.js`

2. When a badge offers different variants of basically the same thing, it’s okay
to put them in the same service class. For example, daily/weekly/monthly/total
downloads can go in one badge, and star rating vs point rating vs rating count
can go in one badge, and same with various kinds of detail about a pull request.
The hard limit (as of now anyway) is _one category per service class_.
to put them in the same service class. For example, daily/weekly/monthly/total
downloads can go in one badge, and star rating vs point rating vs rating count
can go in one badge, and same with various kinds of detail about a pull request.
The hard limit (as of now anyway) is *one category per service class*.

3. If the tests haven’t been split up, split them up too and make sure they
still pass.
still pass.

## Get the route working

1. Disable the legacy service by adding a `return` at the top of
`registerLegacyRouteHandler()`.
`registerLegacyRouteHandler()`.

2. Set up the route for one of the badges. First determine if you can express
the route using a `pattern`. A `pattern` (e.g. `pattern: ':param1/:param2'`) is
Expand All @@ -92,33 +92,33 @@ useful for displaying a badge builder with fields in the front end and
generating badge URLs programmatically.
calebcartwright marked this conversation as resolved.
Show resolved Hide resolved

3. When creating the initial route, you can stub out the service. A minimal
service extends BaseJsonService (or BaseService, or one of the others), and
defines `route()` and `handle()`. `defaultBadgeData` is optional but suggested:
service extends BaseJsonService (or BaseService, or one of the others), and
defines `route()` and `handle()`. `defaultBadgeData` is optional but suggested:

```js
const BaseJsonService = require('../base-json')

class ExampleDownloads extends BaseJsonService {
static get route() {
return {
base: 'example/d',
pattern: ':param1/:param2',
}
}

static defaultBadgeData() {
return { label: 'downloads' } // or whatever
}

async handle({ param1, param2 }) {
return { message: 'hello' }
}
base: 'example/d',
pattern: ':param1/:param2',
}
}

static defaultBadgeData() {
return { label: 'downloads' } // or whatever
}

async handle({ param1, param2 }) {
return { message: 'hello' }
}
}
```

4. We don’t have really good tools for debugging matches, so the best you can do
is run a subset of your tests. To run a single service test, add `.only()`
somewhere in the chain, and run `npm run test:services:trace -- --only=example`.
is run a subset of your tests. To run a single service test, add `.only()`
somewhere in the chain, and run `npm run test:services:trace -- --only=example`.

```js
t.create('build status')
Expand All @@ -133,21 +133,21 @@ t.create('build status')
```

5. Presumably the test will fail, though by examining the copious output, you
can confirm the route was matched and the named parameters mapped successfully.
Since you'll have just run the tests on the old code (right?) you'll know you
haven't inadvertently changed the route (an easy mistake to make).
can confirm the route was matched and the named parameters mapped successfully.
Since you'll have just run the tests on the old code (right?) you'll know you
haven't inadvertently changed the route (an easy mistake to make).

6. If the legacy service had a base URL and you've changed it, you’ll need to
update the tests _and_ the examples. Take care to do that.
update the tests _and_ the examples. Take care to do that.

## Implement `render()` and `handle()`

Once the route is working, fill out `render()` and `handle()`.

1. If there’s a single service, you can implement fetch as a method or a
function at the top of the file. If there are more than one service which share
fetching code, you can put the fetch function in `example-common.js` like this
one for github:
function at the top of the file. If there are more than one service which share
fetching code, you can put the fetch function in `example-common.js` like this
one for github:

<details>

Expand Down Expand Up @@ -211,15 +211,15 @@ module.exports = class PypiBase extends BaseJsonService {
</details>

2. Validation should be handled using Joi. Save this for last. While you're
getting things working, you can use `const schema = Joi.any()`, which matches
anything.
getting things working, you can use `const schema = Joi.any()`, which matches
anything.

3. Substitution of default values should also be handled by Joi, using
`.default()`.
`.default()`.

4. To keep with the design pattern of `render()`, formatting concerns, including
concatenation and color computation, should be dealt with inside `render()`.
This helps avoid static examples falling out of sync with the implementation.
5. To keep with the design pattern of `render()`, formatting concerns, including
concatenation and color computation, should be dealt with inside `render()`.
This helps avoid static examples falling out of sync with the implementation.

## Error handling

Expand All @@ -230,11 +230,11 @@ badge. The cases covered by built-in error handling need not be tested in each
service, and existing tests should be removed.

1. If an external server can't be reached or returns a 5xx status code,
`_requestJson()` along with code in `lib/error-helper.js` will bubble up an
**Inaccessible** error.
`_requestJson()` along with code in `lib/error-helper.js` will bubble up an
**Inaccessible** error.

2. If a response does not match the schema, `validate()` will bubble up an
**InvalidResponse** error which will display **invalid response data**.
**InvalidResponse** error which will display **invalid response data**.

Error handling can also be customized by the service. Alternate messages
corresponding to HTTP status codes can be specified in the `errorMessages`
Expand All @@ -261,16 +261,10 @@ In either case, the service should throw e.g

## Convert the examples

1. Convert all the examples to `pattern`, `namedParams`, and
`staticPreview`. In some cases you can use the `pattern` inherited
from `route`, though in other cases you may need to specify a pattern
in the example. For example, when showing download badges for several
periods, you may want to render the example with an explicit `dt`
instead of `:which`. You will also need to specify a pattern for badges
that use a `format` regex in the route.
1. Convert all the examples to `pattern`, `namedParams`, and `staticExample`. In some cases you can use the `pattern` inherited from `route`, though in other cases you may need to specify a pattern in the example. For example, when showing download badges for several periods, you may want to render the example with an explicit `dt` instead of `:which`. You will also need to specify a pattern for badges that use a `format` regex in the route.

2. Open the frontend and check that the static preview badges look good.
Remember, none of them are live.
Remember, none of them are live.

3. Open up the prepared example URLs in their own tabs, and make sure they work correctly.

Expand Down
You are viewing a condensed version of this merge commit. You can view the full changes here.