Update tutorial to reflect new service layout #2042

chris48s · 2018-09-01T21:13:36Z

Refs #1984

To start off with, I've updated the tutorial to walk the user through writing a service class that extends BaseJsonService instead of defining a badge in server.js using the procedural style. This doesn't complete the documentation refresh - there are still other topics we need to cover/update - but I think this represents a chunk of work which is useful to review.

Next steps:

Update/refresh the service testing docs
Walk though defining a service family with an abstract parent class and multiple badge implementations extending it
Add docs on how to port legacy services

(probably in that order)

while using this as a worked example, I noticed that 'repo' is a bad var name here

chris48s · 2018-09-01T21:14:34Z

doc/TUTORIAL.md

+Description of the code:
+1. As with the first example, we declare strict mode at the start of each file.
+2. Our badge will query a JSON API so we will extend `BaseJsonService` instead of `BaseService`. This contains some helpers to reduce the need for boilerplate when calling a JSON API.
+3. In this case we are making a version badge, which is a common patten. Instead of directly returning an object in this badge we will use a helper function to format our data consistently. There are a variety of helper functions to help with common tasks in `/lib`.


I'd love to be able to link to some useful docs here, but we don't have any (yet). In lieu of a proper internal API reference, is there something more useful we can say here than "there's a bunch of stuff in /lib - good luck finding it ;)"

I'd suggest we survey the few functions that the existing new-style services are using, and list them. You mention renderVersionBadge but we could add e.g. renderLicenseBadge, metric, and whatever else the new-style badges are using now.

shields-ci · 2018-09-01T21:14:43Z

	Warnings
⚠️	This PR modified service code for `gem` but not its test code. That's okay so long as it's refactoring existing code.

	Messages
📖	✨ Thanks for your contribution to Shields, @chris48s!
📖	Thanks for contributing to our documentation. We ❤️ our documentarians!

Generated by 🚫 dangerJS

chris48s · 2018-09-01T21:15:15Z

doc/TUTORIAL.md

+6. As with our previous badge, we need to declare a route. This time we will capture a variable called `gem`.
+7. We can use `defaultBadgeData()` to set a default `color`, `logo` and/or `label`. If `handle()` doesn't return any of these keys, we'll use the default. Instead of explicitly setting the label text when we return a badge object, we'll use `defaultBadgeData()` here to define it declaratively.
+8. Our bage must implement the `async handle()` function. Because our URL pattern captures a variable called `gem`, our function signature is `async handle({ gem })`. We usually seperate the process of generating a badge into 2 stages or concerns: fetch and render. The `fetch()` function is responsible for calling an API endpoint to get data. The `render()` function formats the data for display. In a case where there is a lot of calculation or intermediate steps, this pattern may be thought of as fetch, transform, render and it might be necessary to define some helper functions to assist with the 'transform' step.
+9. The `async fetch()` method is responsible for calling an API endpoint to get data. Extending `BaseJsonService` gives us the helper function `_requestJson()`. Note here that we pass the schema we defined in step 4 as an argument. `_requestJson()` will deal with validating the response against the schema and throwing an error if necessary.


I know we can also pass headers, query string etc here and I'd like to explain this, but there are so many layers of abstraction between here and request I struggled to work out how to clearly explain how we're wrapping it or what subset of the request docs are applicable. Any pointers on how to explain this well?

It would be great to reduce the layers of abstraction. Though, meanwhile, we could say these things:

_requestJson automatically adds an Accept header, checks the status code, parses the response as JSON, and returns the parsed response.

Options can be passed to request, including method, query string, and headers. If headers are provided they will override the ones automatically set by _requestJson. There is no need to specify json, as the JSON parsing is handled by _requestJson. These are all the supported options: https://github.com/request/request#requestoptions-callback

Error messages corresponding to each status code can be returned by passing a dictionary of status codes -> messages in errorMessages.

chris48s · 2018-09-01T21:15:47Z

doc/TUTORIAL.md

+2. The examples property defines an array of examples. In this case the array will contain a single object, but in some cases it is helpful to provide multiple usage examples.
+3. Our example object should contain the following properties:
+    * `title`: Descriptive text that will be shown next to the badge
+    * `urlPattern`: Describe the variable part of the URL using `:param` syntax.


Tangent:

For the moment, lets just document this but I wonder if we could actually find a way to generate this from the url regex and capture vars instead of requiring the user to manually define it. I am reminded of this conversation #1582 (comment)

Yea, I think the solution may be to do this in the opposite direction: we define the urlPattern and automatically generate the regex using url-pattern.

paulmelnikow · 2018-09-02T19:00:44Z

Chris, thanks for taking this on! I responded to your comments though I didn't read it through fully.

I'll be offline for a few days starting tomorrow, so feel free to merge this, and maybe I'll do a set of follow-up edits when I'm back online.

PyvesB · 2018-09-02T20:24:42Z

Don't merge this just yet, I'll give it a read.

PyvesB

Good job, I find the upgraded tutorial very clear! I have left a few minor observations.

PyvesB · 2018-09-02T20:31:43Z

doc/TUTORIAL.md

+Description of the code:
+1. As with the first example, we declare strict mode at the start of each file.
+2. Our badge will query a JSON API so we will extend `BaseJsonService` instead of `BaseService`. This contains some helpers to reduce the need for boilerplate when calling a JSON API.
+3. In this case we are making a version badge, which is a common patten. Instead of directly returning an object in this badge we will use a helper function to format our data consistently. There are a variety of helper functions to help with common tasks in `/lib`.


Typo: "patten"

PyvesB · 2018-09-02T20:42:21Z

doc/TUTORIAL.md

-If there is already a related badge, you may want to place your code next to it.
+### (4.1) Structure and Layout
+
+Service badge code is stored in the `/services` directory in files ending in with `.service.js`


Either "in" or "with" can be removed.

PyvesB · 2018-09-02T20:59:17Z

doc/TUTORIAL.md


-Make sure, you can get the data somehow, best as JSON.
-There might be an API for your service delivering the data.
+The example above was completely static. In order to make a useful service badge we will need to get some data from somewhere. By far the most common case is that we will query an API which serves up some JSON data. In principle we might also query a service which serves another data format.


Could this phrasing be made slightly more open? We've got proper SVG and XML support coming with #2037 and #2031, we don't want to spook out services that don't serve JSON. Something along the lines of:

The most common case is that we will query an API which serves up some JSON data, but other data formats are also fine (e.g. XML).

PyvesB · 2018-09-02T21:07:40Z

doc/TUTORIAL.md

- [merged pull-requests][mergedpr].
+You can also read previous
+[merged pull-requests with the 'service-badge' label](https://github.com/badges/shields/pulls?utf8=%E2%9C%93&q=is%3Apr+label%3Aservice-badge+is%3Amerged)
+to see how other people implemented their badges.

 (2) Setup
 ---------

 I suppose you have [git](https://git-scm.com/) installed.


This is the last remaining bit using first person, could we change it?

Tbh, there is probably a bit of a mashup of styles in the document at this point. The previous tutorial was mostly written in a more "instructive" tone e.g: "you should do this to your code". With the stuff I've added/edited I've mostly gone with a more "inclusive" tone e.g: "we will do this to our code". This is a quite a subtle language style point but it is probably useful to standardise one way or the other. Do you prefer the we/our style?

I don't mind whether it's "you" or "we", but I think we should avoid "I" as it sounds as if one authority is dictating the tutorial.

Right, I see what you mean now - I'll review the "voice" of this section when I go over it again.

chris48s · 2018-09-02T21:46:15Z

Thanks for the comments on this @paulmelnikow and @PyvesB .

As well as the comments provided here @niccokunzmann has submitted this PR to my fork proposing some changes which..vary in scope and I haven't properly read yet.

I'll do another pass over this taking into account the suggestions made in both PRs, resolve the conflicts and update this.

niccokunzmann · 2018-09-03T21:00:05Z

Good idea. You can change it. We will avoid it.

…

On 09/03/2018 09:55 PM, Pyves wrote: ***@***.**** commented on this pull request. ------------------------------------------------------------------------ In doc/TUTORIAL.md <#2042 (comment)>: > -- [CONTRIBUTING.md](../CONTRIBUTING.md) - -You can read - -- previous successful pull-requests to see how other people implemented their badges. - Usually they start with "[add][add-pr]". -- later [pull-requests tagged with `new-badge`][new-badge]. -- [implementations and their commits][blame] in the [server.js][server] file. -- [merged pull-requests][mergedpr]. +You can also read previous +[merged pull-requests with the 'service-badge' label](https://github.com/badges/shields/pulls?utf8=%E2%9C%93&q=is%3Apr+label%3Aservice-badge+is%3Amerged) +to see how other people implemented their badges. (2) Setup --------- I suppose you have [git](https://git-scm.com/) installed. I don't mind whether it's "you" or "we", but I think we should avoid "I" as it sounds as if one authority is dictating the tutorial. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2042 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAieIAo50Q0DxyVbGS7W_qMQJWJ9gFsiks5uXYkugaJpZM4WWXxa>.

This reverts commit e25e748.

many of the most commonly used helpers are in these 4 files

chris48s · 2018-09-04T21:05:04Z

I've pushed some additional edits addressing the various bits of feedback. Thanks, all. @PyvesB maybe you could give this a second look. Cheers

PyvesB

Looks good to me now, thanks for your work on this! 👍

shields-deployment · 2018-09-05T20:34:44Z

This pull request was merged to master branch. This change is now waiting for deployment, which will usually happen within a few days. Stay tuned by joining our #ops channel on Discord!

After deployment, changes are copied to gh-pages branch:

chris48s added 2 commits September 1, 2018 22:07

update the tutorial for new service class layout

9bf61ce

change var name

5fb408e

while using this as a worked example, I noticed that 'repo' is a bad var name here

chris48s added the documentation Developer and end-user documentation label Sep 1, 2018

chris48s commented Sep 1, 2018

View reviewed changes

PyvesB reviewed Sep 2, 2018

View reviewed changes

chris48s and others added 9 commits September 4, 2018 19:58

Revert "Obsolete old tutorial and future TODOs (badges#1983)"

7cf25b8

This reverts commit e25e748.

Merge branch 'revert1983' into tutorial2.0

6475ec2

rewrite section on structure

cfd11e9

describe which super classes can be used and when

aa5a2dc

typo

16816cf

clarify use of other data formats

027c6ce

setup: change voice, add link to install instructions

e33379d

add some more detail on helper functions

4e9848b

many of the most commonly used helpers are in these 4 files

add more detail on passing more args to _requestJson()

64a75f2

chris48s force-pushed the tutorial2.0 branch from 004cea6 to 64a75f2 Compare September 4, 2018 21:02

This was referenced Sep 4, 2018

Tutorial restructuring chris48s/shields#2

Closed

Reduce duplication in badge regex/url patterns #2050

Closed

PyvesB approved these changes Sep 5, 2018

View reviewed changes

chris48s merged commit ef6e601 into badges:master Sep 5, 2018

This was referenced Sep 5, 2018

Update tutorial for new service architecture #1984

Closed

Added basic support for [hockeyapp] #2055

Closed

paulmelnikow mentioned this pull request Jan 8, 2019

gmodstore.com Integration / what badges are in scope for Shields #1946

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update tutorial to reflect new service layout #2042

Update tutorial to reflect new service layout #2042

chris48s commented Sep 1, 2018

chris48s Sep 1, 2018

paulmelnikow Sep 2, 2018

shields-ci commented Sep 1, 2018

chris48s Sep 1, 2018

paulmelnikow Sep 2, 2018

chris48s Sep 1, 2018

paulmelnikow Sep 2, 2018

paulmelnikow commented Sep 2, 2018

PyvesB commented Sep 2, 2018 •

edited

Loading

PyvesB left a comment

PyvesB Sep 2, 2018

PyvesB Sep 2, 2018

PyvesB Sep 2, 2018

PyvesB Sep 2, 2018

chris48s Sep 2, 2018

PyvesB Sep 3, 2018

chris48s Sep 3, 2018

chris48s commented Sep 2, 2018

niccokunzmann commented Sep 3, 2018 via email

chris48s commented Sep 4, 2018

PyvesB left a comment

shields-deployment bot commented Sep 5, 2018

Update tutorial to reflect new service layout #2042

Update tutorial to reflect new service layout #2042

Conversation

chris48s commented Sep 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shields-ci commented Sep 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulmelnikow commented Sep 2, 2018

PyvesB commented Sep 2, 2018 • edited Loading

PyvesB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chris48s commented Sep 2, 2018

niccokunzmann commented Sep 3, 2018 via email

chris48s commented Sep 4, 2018

PyvesB left a comment

Choose a reason for hiding this comment

shields-deployment bot commented Sep 5, 2018

PyvesB commented Sep 2, 2018 •

edited

Loading