[RFC] New API for registering services #963

Daniel15 · 2017-04-26T04:41:43Z

This is a proof-of-concept for what a simplified API for registering services could look like. It's actually not that much code - Essentially it's just a simple facade over all the stuff that exists today. To keep things simple, I just kept everything in server.js for now. I converted over the AppVeyor service as an example.

Some notes:

Code duplication is mostly removed.
- Services no longer need to manually call cache or sendBadge. The service handler simply returns the badge data, and the wrapper around it handles caching
- Service URLs no longer need to manually append .(svg|png|gif|jpg|json) to the end, it's handled automatically
Metadata (name, type, and example URLs) are included so that we can automatically generate the try.html page in the future

Note that this does use async functions, which is a new feature in Node.js 7.x. This means we'd need to bump the Node.js version from 6 to 7. However, I think the code clarity is worth it! The handler has a nice linear flow with a regular try-catch block for handling network errors. We could use promises instead, but async function are the future! 😃

Tested the following URLs on my computer (using Node.js 7.8) and it works nicely:
http://localhost:8080/appveyor/ci/gruntjs/grunt.svg
http://localhost:8080/appveyor/ci/gruntjs/grunt/master.svg
http://localhost:8080/appveyor/ci/kittens/yarn.svg

cc @paulmelnikow
References #948

Daniel15 · 2017-04-26T04:43:23Z

server.js

+
+    const badgeData = getBadgeData(config.type, data);
+    config.handler(params, request.asPromise)
+      .then(serviceData => {


This is still using promise syntax since I didn't want to touch the implementation of cache for now.

Daniel15 · 2017-04-26T04:49:42Z

server.js

+registerService({
+  name: 'AppVeyor',
+  type: 'build',
+  uri: '/appveyor/ci/([^/]+/[^/]+)(?:/(.+))?',


Would be nice to have a simplified way to specify routes (like /appveyor/ci/:user/:project/:branch? or /appveyor/ci/{user}/{project}/{?branch} or something similar) to make the developer experience a bit nicer. Regexes can be pretty inefficient for URLs and abstracting that away would allow a more efficient routing algorithm (eg. using a trie)

However, I'm sticking with small incremental changes and things like that need bigger discussions, so this is a regex for now 😛

Agreed on all counts. Regexes are hard to write and to read.

They are also unidirectional: it's easy to take text and match it with a regex, and hard to use a regex to construct text. A format like /appveyor/ci/:user/:project/:branch? would enable prompting a user for inputs, and constructing a badge URI from that (#777, #466, #701), and similarly would help with an automated tool to create badge URIs (#776).

Upgrading the routing could easily happen in a future pass. 😃

@paulmelnikow - Yeah, I agree. If this was using a simpler format, the examples section below could just be a list of the parameters, rather than the full URL. Example:

uri: '/appveyor/ci/:user/:project/:branch?', examples: [ { user: 'gruntjs', project: 'grunt' }, ],

paulmelnikow · 2017-04-26T17:54:55Z

This looks neat! Look forward to sitting down with it.

Daniel15 · 2017-05-01T17:55:17Z

Thinking about this more, perhaps I should totally avoid async/await and just use promises, so that it'd work today without upgrading to a newer Node.js version. async/await is just promises under the hood, so it could be changed to async/await in the future.

paulmelnikow · 2017-05-02T00:53:35Z

This interface looks really promising. The callbacks really "clean up" well! ✨

I'm leaving you a round of comments.

Thinking about this more, perhaps I should totally avoid async/await and just use promises, so that it'd work today without upgrading to a newer Node.js version.

The handler functions would be a lot of code to rewrite and review twice. We could run an experiment, converting one very popular badge to this async/await form, trying it out in production, and seeing if there are any adverse effects. If async/await are stable enough to be used, I'm inclined to use them. 😃

paulmelnikow · 2017-05-01T23:19:58Z

server.js

+registerService({
+  name: 'AppVeyor',
+  type: 'build',
+  uri: '/appveyor/ci/([^/]+/[^/]+)(?:/(.+))?',


Agreed on all counts. Regexes are hard to write and to read.

They are also unidirectional: it's easy to take text and match it with a regex, and hard to use a regex to construct text. A format like /appveyor/ci/:user/:project/:branch? would enable prompting a user for inputs, and constructing a badge URI from that (#777, #466, #701), and similarly would help with an automated tool to create badge URIs (#776).

Upgrading the routing could easily happen in a future pass. 😃

paulmelnikow · 2017-05-01T23:36:16Z

server.js

-  });
-}));
+  },
+});


For the logistics of this PR, would you mind moving the badge definition into a separate file? It'll be a bit easier to read.

Sure! I think it's self-contained enough that don't that won't be too difficult. I was planning on doing that anyways, as my end goal is to separate out all the services from server.js.

paulmelnikow · 2017-05-01T23:44:32Z

server.js

+      });
+    } catch (e) {
+      return {text: 'inaccessible'};
+    }


Ideally, the service author wouldn't need to implement this try block. It's boilerplate.

Would it be possible to omit it, let the network error bubble up to the calling function, and handle it there?

Yeah good idea, the call to the handler in registerService itself could have a try-catch block. I'll do it!

Actually I just remembered that I do already have a catch block in the registerService, so I don't need to have one here too 😛

paulmelnikow · 2017-05-01T23:53:15Z

server.js

    try {
-      var data = JSON.parse(buffer);
-      var status = data.build.status;
+      result = await request(apiUrl, {


What do you think about switching to node-fetch? It's already well established in browsers and supported natively in the latest version of every major browser. That means going forward, it will be the most familiar to contributors. Every full-stack dev will know fetch, even if they write their server code in Go, and node-fetch is in wide enough use on the server that most Node devs will have encountered it too.

I didn't want to touch any of the core abstractions (caching, fetching, SVG generation, rasterisation, etc) in this pull request. There's lots of fancy caching stuff in request that I don't want to break. I'd like to keep these pull requests small, and keep those changes separate.

paulmelnikow · 2017-05-01T23:58:36Z

server.js

+  handler: async ([repo, branch], request) => {
+    let apiUrl = 'https://ci.appveyor.com/api/projects/' + repo;
+    if (branch != null) {
+      apiUrl += '/branch/' + branch;
    }


Have you experimented with services that contain routing within? For example, CRAN/METACRAN has the version and license within one handler. I'm curious how that could translate. I'm thinking about the examples and the handler code itself.

I haven't yet. I think I'd just model that by having multiple services registered (one for version, one for license) and pull the shared bits into a separate function that both services use.

paulmelnikow · 2017-05-02T00:03:39Z

server.js

    } catch(e) {
-      badgeData.text[1] = 'invalid';
-      sendBadge(format, badgeData);
+      return {text: 'invalid'};


I’ve been reflecting on the way we deal with malformed responses with a mix of careful (e.g. Array.isArray) and broad (try/catch) checks. A try/catch in particular makes it hard for service users (or contributors/maintainers) to differentiate between programmer errors and malformed upstream responses. When writing new code, it creates gotchas, because the try block swallows up all sorts of things like ReferenceErrors.

There’s also a frankensteining of styles: sometimes we have fanatical checks, other times we rely on the try block, and sometimes it's both at the same time.

I wonder if a declarative approach to validation would improve this, and at the same time reduce the number of branches.

Yeah you mentioned this via email. Validation the JSON via Joi or something similar seems like a reasonable future improvement.

paulmelnikow · 2017-05-02T00:06:27Z

server.js

      if (status === 'success') {
-        badgeData.text[1] = 'passing';
-        badgeData.colorscheme = 'brightgreen';
+        return {text: 'passing', colorscheme: 'brightgreen'};


Really like this interface: a function which returns a badge specification. I also like that this one is written so declaratively.

paulmelnikow · 2017-05-02T00:20:28Z

server.js

      if (status === 'success') {
-        badgeData.text[1] = 'passing';
-        badgeData.colorscheme = 'brightgreen';
+        return {text: 'passing', colorscheme: 'brightgreen'};
      } else if (status !== 'running' && status !== 'queued') {


Nit: The logic would be easier to reason about without negation:

if (['queued', 'running'].includes(status)) { return {text: status}; } else if (status === 'success') { return {text: 'passing', colorscheme: 'brightgreen'}; } else { return {text: 'failing', colorscheme: 'red'}; }

The downside of doing it like if (['queued', 'running'].includes(status)) { is that you have one extra array allocation per call 😃 I could flip it so it reads like if (status === 'running' || status === 'queued') though.

paulmelnikow · 2017-05-02T00:41:26Z

server.js

+      params: '/appveyor/ci/gruntjs/grunt/master',
+    },
+  ],
+  handler: async ([repo, branch], request) => {


This is a neat way to break down the regex matches! Very cool!

I wanted to suggest a tweak. To avoid mistakes when code is copied and pasted, and when capture groups are added and removed, would it be better to bind the names closer to the regex?

match: { fragment: '/appveyor/ci/([^/]+/[^/]+)(?:/(.+))?', capture: ['repo', 'branch'], }, handler: async (params, request) => { const { repo, branch } = params; ... }

This would also allow us to validate them automatically. At runtime we could automatically check that the number of identifiers matches the number of capture groups obtained from the regex.

If a handler or a regex is copied without its counterpart, it'll fail obviously when it tries to destructure, instead of confusingly when the labels don't match.

To avoid mistakes when code is copied and pasted, and when capture groups are added and removed

Tests should catch breakages like that though, right? 😄

I really really wish JavaScript had named capture groups, then the regex could look like:

/appveyor/ci/(?<repo>[^/]+/[^/]+)(?<branch>:/(.+))?

And matches would be an object with a repo and branch properties.

(I'm using .NET named capture group syntax for this example)

We could import XRegExp which would allow us to use named capture groups, if you like (fun side note - The developer of XRegExp is my manager at Facebook 😛 ). Alternatively, we could just wait until we refactor the URI parsing, which could also fix how this is handled.

I do like your idea though (specifying names of the capture groups as an array. I think I'll go with that for now, I think it's sufficiently clean.

I like how this came out!

Daniel15

Thanks so much for your comments! I'll iterate on this and make some changes soon.

Daniel15 · 2017-05-02T03:56:32Z

server.js

+registerService({
+  name: 'AppVeyor',
+  type: 'build',
+  uri: '/appveyor/ci/([^/]+/[^/]+)(?:/(.+))?',


@paulmelnikow - Yeah, I agree. If this was using a simpler format, the examples section below could just be a list of the parameters, rather than the full URL. Example:

uri: '/appveyor/ci/:user/:project/:branch?', examples: [ { user: 'gruntjs', project: 'grunt' }, ],

Daniel15 · 2017-05-02T04:01:07Z

server.js

+      params: '/appveyor/ci/gruntjs/grunt/master',
+    },
+  ],
+  handler: async ([repo, branch], request) => {


To avoid mistakes when code is copied and pasted, and when capture groups are added and removed

Tests should catch breakages like that though, right? 😄

I really really wish JavaScript had named capture groups, then the regex could look like:

/appveyor/ci/(?<repo>[^/]+/[^/]+)(?<branch>:/(.+))?

And matches would be an object with a repo and branch properties.

(I'm using .NET named capture group syntax for this example)

We could import XRegExp which would allow us to use named capture groups, if you like (fun side note - The developer of XRegExp is my manager at Facebook 😛 ). Alternatively, we could just wait until we refactor the URI parsing, which could also fix how this is handled.

I do like your idea though (specifying names of the capture groups as an array. I think I'll go with that for now, I think it's sufficiently clean.

Daniel15 · 2017-05-02T04:03:24Z

server.js

+  handler: async ([repo, branch], request) => {
+    let apiUrl = 'https://ci.appveyor.com/api/projects/' + repo;
+    if (branch != null) {
+      apiUrl += '/branch/' + branch;
    }


I haven't yet. I think I'd just model that by having multiple services registered (one for version, one for license) and pull the shared bits into a separate function that both services use.

Daniel15 · 2017-05-02T04:04:43Z

server.js

    try {
-      var data = JSON.parse(buffer);
-      var status = data.build.status;
+      result = await request(apiUrl, {


I didn't want to touch any of the core abstractions (caching, fetching, SVG generation, rasterisation, etc) in this pull request. There's lots of fancy caching stuff in request that I don't want to break. I'd like to keep these pull requests small, and keep those changes separate.

Daniel15 · 2017-05-02T04:05:15Z

server.js

+      });
+    } catch (e) {
+      return {text: 'inaccessible'};
+    }


Yeah good idea, the call to the handler in registerService itself could have a try-catch block. I'll do it!

Daniel15 · 2017-05-02T04:08:00Z

server.js

      if (status === 'success') {
-        badgeData.text[1] = 'passing';
-        badgeData.colorscheme = 'brightgreen';
+        return {text: 'passing', colorscheme: 'brightgreen'};
      } else if (status !== 'running' && status !== 'queued') {


The downside of doing it like if (['queued', 'running'].includes(status)) { is that you have one extra array allocation per call 😃 I could flip it so it reads like if (status === 'running' || status === 'queued') though.

Daniel15 · 2017-05-02T04:08:26Z

server.js

    } catch(e) {
-      badgeData.text[1] = 'invalid';
-      sendBadge(format, badgeData);
+      return {text: 'invalid'};


Yeah you mentioned this via email. Validation the JSON via Joi or something similar seems like a reasonable future improvement.

Daniel15 · 2017-05-02T04:10:34Z

server.js

-  });
-}));
+  },
+});


Sure! I think it's self-contained enough that don't that won't be too difficult. I was planning on doing that anyways, as my end goal is to separate out all the services from server.js.

Daniel15 · 2017-05-02T05:25:20Z

server.js

-    }
-  });
-}));
+registerServices(require('./services/appveyor'));


Eventually I imagine we'll have some sort of "auto registration" for all files in the services directory rather than manually doing it like this.

This is pretty easy to do:

glob.sync(`${__dirname}/services/*.js`).map(name => require(name)).forEach(registerServices);

@paulmelnikow The issue is that we also need to sort the URLs such that longer strings are registered before shorter ones, or enforce that URLs are not substrings of other URLs (and thus will never conflict). Right now there might be some implicit dependencies between badges based on the registration order :/

Right… hmm, why don't we fix that as we're migrating the badges? Probably the few badges where this matters could go in the same file, so they can be kept in order relative to each other. glob will sort alphabetically, so in any case the results should be consistent.

Daniel15 · 2017-05-02T05:27:28Z

services/appveyor.js

@@ -0,0 +1,39 @@
+const loadLogos = require('../lib/load-logos');
+
+module.exports = [{


Services are returned as an array to handle the cases where one service has multiple handlers (eg. NuGet has one URL for release version number, one URL for unstable version number, and URLs for download counts)

Makes sense. How about allowing a single service to be returned as a shorthand?

Added: lib/badge-data has a toArray function. 😄

Daniel15 · 2017-08-13T22:55:29Z

@badges/shields Any other thoughts on this?

paulmelnikow · 2017-09-29T19:44:15Z

Hi! Since I resumed working on Shields, I've been working through the more straightforward PRs and trying to get CI fixed. We're as close as we've ever been to clearing the PR backlog, which would be really great. 🏅

I wrote brand new badges for #1114 and rewrote one for #1112. I encountered a few pain points. The try blocks swallowed errors, the nested request() calls were hard to read, badgeData and query params are clunky, I kept losing my place in the giant server.js, and even though our test setup is much improved, the service tests are opaque when they fail. There are lots of opportunities to improve.

It's a good motivation to pick this up! I'll read again and leave more comments.

Daniel15 · 2017-09-30T07:22:59Z

Thanks @paulmelnikow! I'm happy to iterate on this once I receive feedback 😃

Async functions are pretty reliable in Node.js now that another major version (Node 8) has come out since I originally submitted this PR.

paulmelnikow · 2017-09-30T13:43:21Z

services/appveyor.js

+    {uri: '/appveyor/ci/gruntjs/grunt'},
+    {
+      name: 'Branch',
+      params: '/appveyor/ci/gruntjs/grunt/master',


Is this a typo? One of these examples has a uri key, and the other a params key, for what looks to be the same thing.

Yeah, it's supposed to be uri for both. I originally wanted to do something like:

params: { repo: 'gruntjs/grunt', branch: 'master', }

But it requires us to change how URLs are handled.

paulmelnikow · 2017-09-30T14:02:42Z

services/appveyor.js

+    const data = JSON.parse(result.buffer);
+    const status = data.build.status;
+    if (status === 'success') {
+      return {text: 'passing', colorscheme: 'brightgreen'};


Loving these plain JS object return values! Seems very testable, which is great.

Could you provide a way for defaultBadgeData and the handler to override the label? And maybe think of some clearer property names here. Since a badge has two pieces of text, text is a bit unclear.

paulmelnikow · 2017-09-30T17:46:01Z

server.js

+      params: '/appveyor/ci/gruntjs/grunt/master',
+    },
+  ],
+  handler: async ([repo, branch], request) => {


I like how this came out!

paulmelnikow · 2017-09-30T21:14:52Z

Would you consider an object-oriented version, where each service subclasses a base Service?

In my experience, as these handle()-type functions become more complex, it's very natural to grow them with a class. Classes guide service implementers to build out their code in a more consistent way, which helps with readability across the project. If you look at the badge-specific helpers I broke out in #1109 you'll get a sense of what I mean. Each of these services took a different approach to factoring out their shared code.

The constructor provides a clean way to inject stateful dependencies, as you're doing here. This code includes request, though AFAIK we have four of these right now: we also have regularUpdate, githubAuth.request, and the logger.

It also makes it possible to set up helper objects which use the dependencies, and gives us a lot of flexibility about how these could work. In case it's not clear what I mean, I added a sketch below with one idea of a helper object and helper method.

Finally, I'd prefer we could test a service without standing up a server: in other words, unit test them. The test code could call right into the handler instead of indirecting through HTTP calls. These would be more flexible to set up, better isolated, easier to reason about/debug/instrument, and likely a bit faster to start. A Service superclass provides a nice place to put registerServices, and to break out parts as needed to support testing.

class AppVeyor extends Service {
  constructor (context) {
    super(context);
    this.vendor = new JsonVendor(context, {
      validationSchema: Joi.object().keys({
        data: Joi.object().keys({
          status: Joi.equal('success', 'running', 'queued', 'failing').isRequired()
        }).isRequired()
      }),
      notFoundMessage: 'project not found or access denied',
    });
  }

  static getUri({ repo, branch }) {
    ...
  }

  async handler (params) {
    const data = await this.vendor.request(Appveyor.getUri(params), ...);
    ...
  }
}
Object.assign(AppVeyor, {
  dependencies: JsonVendor.dependencies,
  // `name` can default to introspecting from the class name
  type: 'build',
  ...
});

Thoughts?

Daniel15 · 2017-10-01T23:28:04Z

Using classes is an interesting idea. I was trying to keep the code relatively light, but using classes should be fine. I might not have time to get back to this for a few weeks, but I'll definitely come back to it soon.

platan · 2017-10-18T19:46:19Z

services/appveyor.js

+  type: 'build',
+  uri: {
+    format: '/appveyor/ci/([^/]+/[^/]+)(?:/(.+))?',
+    capture: ['repo', 'branch']


I really like this idea. Group names definition is right below regex. Simple and useful implementation of named capturing groups.

I feel like using type as property name might be misleading to developers. In the end, we all know that the type will define the left label. So, why no just call that property leftLabel or title ?

The fact that it's used as the left label is an implementation detail. type is really the "group" that it falls under on the https://shields.io/ website. I could call it group or category instead maybe?

I understand 😄 But I feel like forcing the semantic meaning of the left label as a "group" is too restrictive. What if users want to make tags that have a left label that is not an actual group ? As an example, yesterday I made (okay, it's not a great example, but it was quite fun) a badge that shows the weather in a given town (I haven't PR's it, I mean, not like anyone would really use it 😛 ). I would not consider that "weather" is a type of badge. Anyways, if the goal is to keep it as a "type" semantic, I would name category as I think developers could think that type is related to some sort of generic badge type ? (which I guess is you goal ^^).

Agree category is good, since it suggests it's taxonomical in nature. Often the category is the same as the default label, so I like this way of handling it.

It's not a bad idea to provide a way to override the default label when it's different from the category. For example, the Github stars badge is in the "social" category but the default label is "Stars."

Daniel15 · 2017-10-18T19:57:39Z

Now that I'm back in the USA, I might try to look at this again over the weekend :) Sent from my phone.

…

On Oct 18, 2017 12:46 PM, "Marcin Mielnicki" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In services/appveyor.js <#963 (comment)>: > @@ -0,0 +1,39 @@ +const loadLogos = require('../lib/load-logos'); + +module.exports = [{ + name: 'AppVeyor', + type: 'build', + uri: { + format: '/appveyor/ci/([^/]+/[^/]+)(?:/(.+))?', + capture: ['repo', 'branch'] I really like this idea. Group names definition is right below regex. Simple and useful implementation of named capturing groups. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#963 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFnHQEA9Lcrmi6B9tmm3AsUBiKFrL_pks5stlWOgaJpZM4NIWrw> .

platan · 2017-10-18T20:13:27Z

server.js

+    );
+    config.handler(namedParams, request.asPromise)
+      .then(serviceData => {
+        let text = badgeData.text;


I guess we can have const here.

paulmelnikow · 2017-10-18T20:44:02Z

Now that I'm back in the USA, I might try to look at this again over the weekend :)

That would be awesome!

Daniel15 · 2017-10-23T03:56:45Z

@paulmelnikow - I was thinking about this a bit, and am wondering if we do actually need a base class...

A Service superclass provides a nice place to put registerServices, and to break out parts as needed to support testing.

Usually I find that mocking frameworks are sufficient to support testing, as long as the code has been broken out into a number of small modules. JavaScript testing frameworks let you mock anything (so for example we could mock lib/request-handler to totally mock the outbound request mechanism).

Do you still think there's value in having a base class? Would we have a single instance of the class for the entire run time of the app, or would we instantiate one instance per request (and thus allow them to have instance data specific to the one request)?

paulmelnikow · 2017-10-26T21:40:20Z

What I meant about breaking out parts of registerServices for testing is that the function does a lot of things. I'd like a way to pass in a URI and maybe query parameters, and get badge data. To do this clearly would require factoring that code out of registerServices, one way or another.

I'm thinking one instance per run of the app. I've looked at a lot of these services and while they need a way to invoke helpers with a parameter or two, none of them involve complex request state.

There's a lot to be said for a common pattern of initializing helper objects and factoring out helper functions. Experience says classes really do help people write more uniform code.

(Sorry, just saw this comment!)

paulmelnikow · 2017-11-10T04:49:26Z

Hey! I thought I would check in about this! Do you think you'll have time to look at this soon?

Daniel15 · 2017-12-03T22:23:27Z

@paulmelnikow - I think I got all your changes! I also added a very basic unit test 😃

Turns out I didn't even need to think about the logo stuff yet, as it seems like the AppVeyor badge no longer shows the logo by default. Because of that, I just removed all the logo-related changes from this PR

paulmelnikow · 2017-12-04T00:25:28Z

services/base.spec.js

+    assert.equal(mockCamp.route.getCall(0).args[0].toString(), expectedRouteRegex);
+  });
+
+  it('handles the request', async () => {


Aw, love these async tests with async/await!

Yeah I'm happy that Mocha allows test cases to return promises. Async/await is just a fancy syntax for promises.

I'm used to Jest, so I had to keep the Mocha docs open while writing these tests. They're similar but there's subtle differences (eg. Mocha doesn't have a mocking/stubbing framework built in and you need to use Sinon, whereas Jest includes mocking out-of-the-box).

paulmelnikow · 2017-12-04T00:26:47Z

Looks great!

Daniel15 · 2017-12-04T00:51:08Z

Sorry for not fixing the lint issues! Thanks for looking at it :)

paulmelnikow · 2017-12-04T02:16:04Z

Would you like the honor of merging this? 😁

Daniel15 · 2017-12-04T02:17:00Z

Sure! Let's do it 😄

paulmelnikow · 2017-12-04T02:21:53Z

Could you do that with squash instead?

Daniel15 · 2017-12-04T02:23:07Z

Oops, I thought I selected the squash option... Didn't realise it wasn't the default for this repo (I'm used to it being the default option). I can figure out how to squash the commits and force push it.

paulmelnikow · 2017-12-04T02:37:00Z

Github doesn't let you set a default. We could disable rebasing though. I wouldn't mind doing that. The times I've used rebase were when two things were done in one PR but separate commits, and the history added a lot of context. Rare enough that it's probably worth not having to think about it!

Daniel15 · 2017-12-04T02:38:42Z

Today I learned how to squash commits :) c7f03fd

@Daniel15

…1543 This merges the `node-8` branch. The heavy lift was by @Daniel15 with refactoring from me and a patch by @RedSparr0w. * New API for registering services (#963) * Disable Node 6 tests on node-8 branch (#1423) * BaseService: Factor out methods _regex and _namedParamsForMatch (#1425) - Adjust test grouping - Rename data -> queryParams, text -> message * BaseService tests: Use Chai (#1450) * BaseService: make serviceData and badgeData explicit and declarative (#1451) * fix isValidStyle test (#1544) * Run tests in Node 9, not Node 6 (#1543)

Daniel15 commented Apr 26, 2017

View reviewed changes

Daniel15 mentioned this pull request Apr 26, 2017

Modernization + code cleanup ideas #948

Closed

paulmelnikow added the developer-experience Dev tooling, test framework, and CI label Apr 26, 2017

paulmelnikow reviewed May 2, 2017

View reviewed changes

Daniel15 commented May 2, 2017

View reviewed changes

Daniel15 force-pushed the new-api branch from 5f63314 to a57827d Compare May 2, 2017 05:24

Daniel15 commented May 2, 2017

View reviewed changes

perceptron8 mentioned this pull request Jul 2, 2017

add Opera Add-ons support #990

Closed

paulmelnikow reviewed Sep 30, 2017

View reviewed changes

paulmelnikow mentioned this pull request Oct 2, 2017

Move remaining helper functions to lib/ #1109

Merged

platan reviewed Oct 18, 2017

View reviewed changes

This was referenced Oct 26, 2017

Repository refactoring #1210

Closed

Allow user overrides to always take priority #1234

Closed

paulmelnikow mentioned this pull request Nov 10, 2017

Add [Liberapay] #1251

Merged

10 tasks

Daniel15 added 2 commits December 3, 2017 13:33

New API for registering services - v2

11e8f19

Code review comments, and add basic unit test

1faa976

Daniel15 force-pushed the new-api branch from 398b783 to 1faa976 Compare December 3, 2017 22:18

Daniel15 changed the base branch from node-8 to master December 3, 2017 22:20

Daniel15 changed the base branch from master to node-8 December 3, 2017 22:20

Remove unneeded load-logos changes

1c2b3ce

paulmelnikow reviewed Dec 4, 2017

View reviewed changes

paulmelnikow added 4 commits December 3, 2017 19:31

Get lint passing with async/await

2330181

Clear some more build errors

dbf3c0f

Oops, forgot server does not work in Node 9

160a4a5

Let’s make life simple and drop Node 9 for now

b23784c

Daniel15 merged commit 1eef462 into badges:node-8 Dec 4, 2017

NZSmartie mentioned this pull request Dec 4, 2017

Customise AppVeyor Test 'passed', 'failed', 'skipped' status #1321

Closed

This was referenced Dec 8, 2017

try catch finally sendBadge #1356

Closed

The Great Service Rewrite 🏆 #1358

Closed

paulmelnikow mentioned this pull request Dec 26, 2017

Add alternative static badge #1395

Closed

paulmelnikow mentioned this pull request Aug 23, 2018

add examples for documentation contributions #1962

Merged

calebcartwright mentioned this pull request Jan 25, 2019

Help wanted to finish refactoring services 🏆 #2863

Closed

paulmelnikow mentioned this pull request May 6, 2019

RFC for injecting server secrets + refactoring auth code [azuredevops bintray bitbucket] #3410

Closed

paulmelnikow mentioned this pull request Nov 7, 2019

Azure DevOps Artifacts Nuget Feed Badge, run [nuget myget chocolatey resharper] #4302

Closed

		@@ -0,0 +1,39 @@
		const loadLogos = require('../lib/load-logos');

		module.exports = [{

[RFC] New API for registering services #963

[RFC] New API for registering services #963

Conversation

Daniel15 commented Apr 26, 2017

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 Apr 26, 2017

Daniel15 commented May 1, 2017

paulmelnikow commented May 2, 2017

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Daniel15 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

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

paulmelnikow Sep 30, 2017 • edited Loading

Choose a reason for hiding this comment

Daniel15 commented Aug 13, 2017

paulmelnikow commented Sep 29, 2017

Daniel15 commented Sep 30, 2017

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

paulmelnikow commented Sep 30, 2017

Daniel15 commented Oct 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gempain Oct 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Daniel15 commented Oct 18, 2017 via email

Choose a reason for hiding this comment

paulmelnikow commented Oct 18, 2017

Daniel15 commented Oct 23, 2017

paulmelnikow commented Oct 26, 2017

paulmelnikow commented Nov 10, 2017

Daniel15 commented Dec 3, 2017

Choose a reason for hiding this comment

Daniel15 Dec 4, 2017 • edited Loading

Choose a reason for hiding this comment

paulmelnikow commented Dec 4, 2017

Daniel15 commented Dec 4, 2017

paulmelnikow commented Dec 4, 2017

Daniel15 commented Dec 4, 2017

paulmelnikow commented Dec 4, 2017

Daniel15 commented Dec 4, 2017

paulmelnikow commented Dec 4, 2017

Daniel15 commented Dec 4, 2017

paulmelnikow Sep 30, 2017 •

edited

Loading

gempain Oct 27, 2017 •

edited

Loading

Daniel15 Dec 4, 2017 •

edited

Loading