Add [Liberapay] #1251

mattbk · 2017-11-07T06:11:20Z

For top-level entities.

paulmelnikow

Looks like a good start!

This will need service tests to be merged. Let me know if you have questions about that.

paulmelnikow · 2017-11-07T13:08:48Z

server.js

+          badgeData.colorscheme = 'green';
+        } else {
+          badgeData.colorscheme = 'brightgreen';
+        }


You should be able to accomplish this using colorScale.

colorScale([0, 10, 100])(receiving)

I'm not sure how to implement that, is there an example somewhere?

paulmelnikow · 2017-11-07T13:10:04Z

server.js

+        sendBadge(format, badgeData);
+      }
+    } catch(e) {
+      badgeData.text[1] = apiUrl;


Why does this put the API URL on the badge?

That's a testing mistake, looks like I didn't push my latest change.

paulmelnikow · 2017-11-07T13:11:15Z

server.js

+      var data = JSON.parse(buffer);
+      // Avoid falsey checks because amounts may be 0
+      var receiving = data.receiving.amount;
+      if (!isNaN(receiving)) {


It's a bit odd to use isNaN on a JSON value. Can you invert this and test receiving === null instead?

mattbk · 2017-11-07T15:22:24Z

@paulmelnikow thanks for the early review!

elegaanz

Thank you for this.

elegaanz · 2017-11-07T19:50:35Z

server.js

+      // Avoid falsey checks because amounts may be 0
+      if (type == 'gives') {
+        var value = data.giving.amount;
+        } else {


I guess there is an indentation problem here. And maybe you could make this part simple with a ternary expression, like: var value = type === 'gives' ? data.giving.amount : data.receiving.amount.

Yes. Although I may go with a switch since I intend to add more options.

mattbk · 2017-11-08T06:01:25Z

Currently:

ghost · 2017-11-08T17:34:50Z

This is an awesome idea/badge. When will it become usable ?

mattbk · 2017-11-08T17:45:28Z

Hoping to finish it up today or tonight, not sure how long review and deploy will take after that.

mattbk · 2017-11-08T21:48:19Z

/home/travis/build/badges/shields/server.js
1073:22 error Missing semicolon semi
1081:15 error 'badgeData' is already defined no-redeclare
1084:15 error 'badgeData' is already defined no-redeclare
1087:15 error 'badgeData' is already defined no-redeclare
1104:67 error Unnecessary escape character: / no-useless-escape
1107:17 error 'value' is already defined no-redeclare
1111:17 error 'value' is already defined no-redeclare
1115:17 error 'value' is already defined no-redeclare
1116:17 error 'currency' is already defined no-redeclare

mattbk · 2017-11-08T22:19:46Z

Not done yet, tests are weak.

mattbk · 2017-11-09T06:34:00Z

Looks like a timeout on Travis? Otherwise this is ready for review aside from not knowing the best way to use colorScale().

paulmelnikow · 2017-11-09T14:59:40Z

PR: badges/shields#1251
Error: 522 Origin Connection Time-out
    at Error (native)
    at fetch.then.res (/home/travis/build/badges/shields/service-tests/runner/pull-request-services-cli.js:27:15)
    at process._tickCallback (internal/process/next_tick.js:109:7)

That particular test failure is due to a Shields production issue #1245. We use the Shields server to fetch the PR title from github.

paulmelnikow · 2017-11-09T15:00:53Z

Is anyone else following this up for writing docs for colorScale? There are three good examples to generalize from, in lib/color-formatters.spec.js.

mattbk · 2017-11-15T20:46:37Z

Made changes, don't know if you saw that.

paulmelnikow · 2017-11-18T16:54:08Z

Hey, thanks for the heads up.

Could you find a project you're confident will have patrons and contributions for the foreseeable future? It's okay if we have to switch it to another project down the line.

mattbk · 2017-11-19T02:32:24Z

It works the same for projects and individuals. Changaco is in charge of Liberapay, so I assumed he would be pretty stable. Or am I misunderstanding the question?

paulmelnikow · 2017-11-19T15:34:51Z

Ah, I'm referring to this discussion:

#1251 (comment)
#1251 (comment)

I'd like to change the regex so it rejects 0. It gives more confidence that passing tests == working badge.

…iberapay

mattbk · 2017-11-20T18:56:17Z

n.b. npm run test:services -- --only-travis --only=liberapay

mattbk · 2017-11-20T19:46:26Z

This tested with an account with zero patrons and 0.00 receiving. Will now push corrected version.

mattbk · 2017-11-20T20:51:31Z

The order of the last push and last commented should be reversed.

paulmelnikow

This looks great. I left couple minor comments. Would love to get this merged!

paulmelnikow · 2017-11-30T18:36:38Z

server.js

+      case 'goal':
+          badgeData = getBadgeData('goal progress', data);
+          break;
+      }


How about:

const label = { receives, gives, patrons, goal: 'goal progress', }[type]; const badgeData = getBadgeData(label, data);

Seems like it would be a good idea to lock down the type part of the regex to accept only these strings. That way you don't need to handle the errors.

Sounds good. Fixed in 47803f7.

Could you edit the regex too, so it only accepts the four acceptable values?

Weird I didn't see this. Think I fixed in f0a84f7.

paulmelnikow · 2017-11-30T18:38:30Z

service-tests/liberapay.js

+  .get('/patrons/Liberapay.json')
+  .expectJSONTypes(Joi.object().keys({
+    name: 'patrons',
+    value: Joi.string().regex(/^[1-9]+/)


Is there any reason not to use isMetric?

I don't believe so. Fixed in 66fc147.

…iberapay

…o add-liberapay

paulmelnikow · 2017-12-06T00:26:12Z

Could you address the last pending comment?

#1251 (comment)

paulmelnikow · 2017-12-06T04:01:11Z

Fabulous! Thanks so much!

mattbk · 2017-12-22T17:20:57Z

Did I do something wrong? I'm still not seeing this listed at shields.io.

paulmelnikow · 2017-12-22T17:28:47Z

It's waiting for deploy. Deploys usually happen every 1–3 weeks. Thaddée, who has limited time on this project, is the only sysadmin. He's working on giving me access to deploy and logs, but doing so is complicated because the hosting account (and maybe the servers too) are shared with other services he runs. It's also been held up recently because of #1314.

mattbk · 2017-12-22T18:07:43Z

Thanks for the explanation. Didn't know how often deployment happened.

Add receiving badge.

41a508d

paulmelnikow reviewed Nov 7, 2017

View reviewed changes

paulmelnikow added the service-badge New or updated service badge label Nov 7, 2017

mattbk added 4 commits November 7, 2017 09:39

Fix invalid.

b30329c

Stub out giving vs receiving.

eda1c32

Finish gives vs receives.

6792bf0

Fix null check.

b609b08

mattbk mentioned this pull request Nov 7, 2017

Badge at Shields.io liberapay/liberapay.com#775

Closed

elegaanz reviewed Nov 7, 2017

View reviewed changes

Add more badges.

7131f27

mattbk added 2 commits November 8, 2017 14:20

Simplify.

b549b84

Start tests.

d61e174

mattbk added 3 commits November 8, 2017 16:01

Fix minor issues.

7891d70

Avoid redeclares.

b0589c6

Rm trailing space.

923bb5e

mattbk added 4 commits November 8, 2017 17:30

Add more tests.

48b8dae

Shelve.

e3df852

Deal with anonymous values.

850457b

Fix no goal/anonymous test.

8cd7e04

paulmelnikow changed the title ~~WIP: Add Liberapay~~ WIP: Add [Liberapay] Nov 9, 2017

Add colorScale.

d606cfa

mattbk added 2 commits November 9, 2017 17:12

Make suggested changes.

0bbceb9

Fix tests.

09713a8

mattbk changed the title ~~WIP: Add [Liberapay]~~ Add [Liberapay] Nov 10, 2017

Merge branch 'master' of https://github.com/badges/shields into add-l…

7b87230

…iberapay

mattbk added 3 commits November 20, 2017 13:34

Suggestions, but regex is wrong for values less than 1.00.

4faadc3

Fixed regex. Two tests will fail.

98ef2f2

Fix tests.

9d3f86f

paulmelnikow reviewed Nov 30, 2017

View reviewed changes

mattbk added 4 commits November 30, 2017 21:58

Merge branch 'master' of https://github.com/mattbk/shields into add-l…

322d71a

…iberapay

Merge branch 'add-liberapay' of https://github.com/mattbk/shields int…

14374f0

…o add-liberapay

Replace case statement.

47803f7

Use isMetric for npatrons.

66fc147

espadrine force-pushed the master branch from 203d6d5 to 7e668c7 Compare December 3, 2017 18:06

mattbk mentioned this pull request Dec 6, 2017

public.json returns more than two decimal places liberapay/liberapay.com#831

Closed

Limit regex.

f0a84f7

paulmelnikow merged commit 2028251 into badges:master Dec 6, 2017

mattbk deleted the add-liberapay branch December 22, 2017 18:07

Changaco mentioned this pull request Mar 5, 2018

Caching of API responses #1545

Closed

Add [Liberapay] #1251

Add [Liberapay] #1251

Conversation

mattbk commented Nov 7, 2017 • edited Loading

paulmelnikow 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

mattbk commented Nov 7, 2017

elegaanz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattbk commented Nov 8, 2017

ghost commented Nov 8, 2017

mattbk commented Nov 8, 2017

mattbk commented Nov 8, 2017

mattbk commented Nov 8, 2017

mattbk commented Nov 9, 2017

paulmelnikow commented Nov 9, 2017

paulmelnikow commented Nov 9, 2017

mattbk commented Nov 15, 2017

paulmelnikow commented Nov 18, 2017

mattbk commented Nov 19, 2017

paulmelnikow commented Nov 19, 2017

mattbk commented Nov 20, 2017

mattbk commented Nov 20, 2017

mattbk commented Nov 20, 2017 • edited Loading

paulmelnikow 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

mattbk Dec 1, 2017 • edited Loading

Choose a reason for hiding this comment

paulmelnikow commented Dec 6, 2017

paulmelnikow commented Dec 6, 2017

mattbk commented Dec 22, 2017

paulmelnikow commented Dec 22, 2017

mattbk commented Dec 22, 2017

mattbk commented Nov 7, 2017 •

edited

Loading

mattbk commented Nov 20, 2017 •

edited

Loading

mattbk Dec 1, 2017 •

edited

Loading