[badge-json] new "badge-json" service #1525

bkdotcom · 2018-02-25T06:40:26Z

route: /^/json/(https?.+).(svg|png|gif|jpg|json)$/
added documentation to usage.js (with json-badge-maker)

a few tweaks to documentation

better separate "static", "dynamic", and "json"
added "style" parameter to parameter list & moved rendered styles from above the list to below
removed the ginormous margin-left: 25% style given to UL -> now applies to ul.options (there was only one UL in in documentation)
renamed duplicate "miscellaneous" id to "miscellaneous2"

Screenshot of updated docs:

route: /^\/json\/(https?.+)\.(svg|png|gif|jpg|json)$/ added documentation to usage.js (with json-badge-maker) tweaked usage.js oh-so-slightly to better separate "static", "dynamic", and "json" added "style" parameter to parameter list & moved rendered styles from above the list to below removed the ginormous margin-left: 25% style from ul -> now applies to ul.options (was only one UL in in documentation) renamed duplicate "miscellaneous" id to "miscellaneous2"

shields-ci · 2018-02-25T06:41:14Z

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

Generated by 🚫 dangerJS

bkdotcom · 2018-02-25T12:57:44Z

question: how do I write a unit test that tests the style/template, colorA, & colorB.
All the current tests only seem to test against the json response (which only contains name & value)

platan · 2018-02-25T14:32:13Z

@bkdotcom There is a way to verify colors in service tests using this approach (style=_shields_test):

shields/service-tests/github.js

Lines 38 to 40 in 8ad176d

    
           t.create('License for repo without a license') 
        
             .get('/license/badges/badger.json?style=_shields_test') 
        
             .expectJSON({ name: 'license', value: 'missing', colorB: colorsB.red });

Is it helpful?

bkdotcom · 2018-02-25T19:31:37Z

@platan Thanks!

so.. I committed an updated unit test and now one of the circleci tests fails with a seemingly unrelated
should produce colorscheme PNG badges
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

sunspots? Is there a way to rerun the test without a commit?

platan · 2018-02-25T19:45:32Z

You can close and reopen PR to run CI again. Does anyone know a better way?

RedSparr0w · 2018-02-26T00:25:30Z

Thanks for the PR,
This is a pretty cool idea!
I'll have to have a look at it later this week when i have a bit more time 😄

I just re-ran the test,
Not sure of a way (other than mentioned above) to re-run your own tests,
I think it may be maintainers only.

added notes to "dynamic" badge (what the params are)

bkdotcom · 2018-02-28T13:39:20Z

thought 💡:

Right now the JSON can contain "colorA" and "colorB" (which matches the optional url param names).

What if instead, the JSON uses less ambiguous "labelBackgroundColor" and "valueBackgroundColor" (or "labelBgColor" / "valueBgColor").. The thought being that at some point the service could also specify "labelTextColor" / "valueTextColor (or instead something like boolean "labelTextInvert" / "valueTextInvert" which would use black text rather than white)

paulmelnikow · 2018-03-04T00:05:51Z

This is a really interesting idea! I like it. I'll leave some comments on the specifics.

@bkdotcom Seems like you're up on React! I wonder if we could take this opportunity to continue to break features out onto separate pages. What do you think about moving all the docs on Badge JSON to a new modal? There's a lot of detail and I think it would be good to have a chance to explain it further, and also why you might choose it when compared to other options.

Typically the way we've handled this in the past is to have services redirect to our static badge. That does have an advantage over this badge JSON approach, which is that Shields doesn't have to make any network requests, making it much more performant on our end. However, this syntax is really nice. Much clearer.

We've talked often about a more computer-friendly version of the static badge. Maybe we could take the badge JSON format developed here, and provide a second way to use it at something like https://shields.io/static.svg?badgeData=encoded-json-here.

That way, services only use this one (that requires us to fetch the data) if they can't easily pull off the redirect.

Does that make sense?

paulmelnikow · 2018-03-04T00:06:58Z

Not sure of a way (other than mentioned above) to re-run your own tests,

You can push a "null edit" – basically a commit that changes whitespace or some other meaningless thing. If master has moved, merging that would accomplish it too.

paulmelnikow · 2018-03-04T00:20:39Z

Right now the JSON can contain "colorA" and "colorB" (which matches the optional url param names).

What if instead, the JSON uses less ambiguous "labelBackgroundColor" and "valueBackgroundColor" (or "labelBgColor" / "valueBgColor").. The thought being that at some point the service could also specify "labelTextColor" / "valueTextColor (or instead something like boolean "labelTextInvert" / "valueTextInvert" which would use black text rather than white)

I'd like to move away from colorA and colorB, which aren't the most descriptive. There are some other names I'd like to tweak in here, too. Here are my thoughts:

value -> message for the right hand text. This is what we use for the badge data in the node-8 branch where we've created a new framework for rewriting the services, once the server gets upgraded to Node 8.
colorB -> color. Same. When people think of the badge color, this is it.
colorA -> labelColor. This is rarely overridden. It's never overridden in Shields itself.
valueClass -> severity. I really like the way you formalized this! Though I think severity is a more descriptive name. Would love to use that in our code, too, because it adds some good semantics. Come to think of it, I wonder if we should back this out for a moment, so we can give all the values some more careful thought. I'm curious how we're using colors throughout the repo now, and if it corresponds with the way you've labeled it here. Once an API is put out there, it becomes much more difficult to change, so it's good to be thoughtful about it…
version. Could you add this? Ideally I'd like to require people to pass 1. Then when we want to do a version 2 we can easily distinguish old requests from new ones.
isSocial – Any reason we couldn't keep using style for this? I don't like to introduce new syntax for the same thing without good reason.

paulmelnikow

Love this!

paulmelnikow · 2018-03-04T00:22:37Z

server.js

+        return;
+      }
+      try {
+        let data = JSON.parse(buffer);


Could you move this transformation code into its own function in lib/, and make a list there of the supported parameters? It's great to have it in the web page, but feel like it ought to live with the code, too. By breaking this out into a separate (pure) function, it becomes very easy to test all this logic using data-driven tests. You'll see we've a bunch of these kind of tests now, using sazerac.

paulmelnikow · 2018-03-04T00:32:18Z

server.js

@@ -7545,6 +7545,71 @@ camp.route(/^\/maven-metadata\/v\/(https?)\/(.+\.xml)\.(svg|png|gif|jpg|json)$/,
    });
 }));

+// badge-json url
+camp.route(/^\/json\/(https?.+)\.(svg|png|gif|jpg|json)$/,


As I mentioned in #1525 (comment) let's make two or maybe three ways of leveraging the JSON functionality.

I'd prefer we add support for three endpoints:

/badge/dynamic/json-badge-data/https/example.com/info.json.svg (as an alternative to /json`)

/badge/dynamic/json-badge-data.svg?url=https%2Fexample.com%2Finfo.json

/badge/static.svg?badgeData=...

These support the more efficient static route for servers capable of building a redirect. That route can also be used in a dynamic web page, which is really cool! Simultaneously they make the "static" and "dynamic" distinctions very clear.

Good idea!

sidenote

I'm not much of a fan of the https/example.com endpoints,
just feels strange without the https://:
/badge/dynamic/json-badge-data/https://example.com/path/to.json.svg
/badge/dynamic/json-badge-data/https/example.com/path/to.json.svg
but we have a few that way already so for the sake of consistency it's probably best to continue that pattern.

Yea… I find it a little weird too. Though, I think a colon in a path is supposed to be URL encoded. Maybe we could just make the scheme optional and default to https? That way it would disappear in most cases…

paulmelnikow · 2018-03-04T00:35:30Z

service-tests/badge-json.js

+  .expectJSON({
+    name: 'test',
+    value: 'success',
+    colorB: '#C001E0' // colorB param


It would be good to keep some tests here, though it'll be possible to write some of these more simply as unit tests if the transformation logic is moved from server.js to a function in lib/.

paulmelnikow · 2018-03-04T16:31:07Z

Realized the semantic color discussion has been started: #1522.

bkdotcom · 2018-03-05T17:40:23Z

Re the discussion that happened over the weekend

I'll try to get to the requested modifications this week
May hand some of it off... probably bit of more than I wish to chew :|

Seems like you're up on React!

Nope... just proficient at copy/paste :)

Re renaming keys:

proposed	thought(s)
value -> message	✓
colorB -> color	✓
colorA -> labelColor	✓
valueClass -> severity	✓✓ Much better. I struggled to come up with a term.. "severity" was escaping me
version	✓
isSocial ?	my reasoning: I though it weird for the service to specify the style (flat, plastic, for-the-badge, flat-square, etc) this is usually something that the user-of-the-badge would want to specify (and not give control to the service) the social "style" (template) is a different beast.. with the ability to specify the link params and/or not even have a value/"message" ie "isSocial" also conveys the type of service (or type of badge) (the semantic thing again), not just how to style the badge) /ramble

The json format as currently implemented also supports "link"

{
   "link" : ["http://www.example.com/labellink", "http://www.example.com/messageLink"]
}

after my isSocial ramble:
two separate keys may be easier to use / keep everything flat
labelLink & messageLink
would also make it easier (more straightforward) to add a link for only the message

better:?

{
    "labelLink" : "http://www.example.com/labellink", 
    "messageLink" : "http://www.example.com/messageLink"
}

paulmelnikow · 2018-03-06T06:35:48Z

my reasoning: I though it weird for the service to specify the style (flat, plastic, for-the-badge, flat-square, etc)
this is usually something that the user-of-the-badge would want to specify (and not give control to the service)
the social "style" (template) is a different beast.. with the ability to specify the link params and/or not even have a value/"message"
ie twitter
"isSocial" also conveys the type of service (or type of badge) (the semantic thing again), not just how to style the badge)

That's a really interesting distinction to make between the user of the badge and the data provider. It's a concern unique to the dynamic json-badge-data badges, since redirects to static badges are probably not going to be customizable. I really like your idea that the user should be able to customize the presentation.

Also agreed, you're onto something about isSocial being a useful semantic difference. In fact ?style=social is conflating appearance and semantics. Come to think of it, it might be clearer to have certain badges default to social style unless someone requests an ordinary / non-social badge with ?social=false. And asking for ?social if they want it, e.g. as someone might on the Discord badge. I think some of our default should change, and also that we should try to harmonize the way we handle this across the API. Could I ask that we defer it for now, and then reintroduce that piece when we can think about it more holistically?

Re links, the most likely case is a single link for the whole badge. It didn't occur to me it was possible to link the message and not the label. I agree an array of links is not at all intuitive.

Another option might be:

Links both sides:

"link": "http://example.com"

Links one side:

"link": { "message": "http://example.com" }

Links one side:

"link": { "label": "http://example.com/1", "message": "http://example.com/2" }

I quite like the elegance of that. However, labelLink and messageLink would translate better to query parameters.

paulmelnikow · 2018-03-21T01:54:00Z

Hi! Before you pick this up again, could you please merge in master, and move service-tests/badge-json.js into services/, following the new scheme from #1563?

paulmelnikow · 2018-11-15T19:08:54Z

There are a lot of open PRs with code that needs reviewing. To help the maintainers identify the actionable PRs that are on a critical path, I'm marking this PR "on hold." It's not dead and can or will be picked up at some point.

Work on this PR is blocked until a port of the existing dynamic badge goes through. I'm prioritizing that behind #2284 which may inform it and will create some conflicts in server.js. This PR then needs to be worked on a bit, in particular with some thought to the API design, and then rewritten using new-style services.

paulmelnikow · 2018-12-05T00:40:10Z

As I mentioned in #2259 I'm working on reviving this.

The PR can't be opened until #2399 is merged.

@bkdotcom

This reimplements the idea @bkdotcom came up with in #1519, and took a stab at in #1525. It’s a really powerful way to add all sorts of custom badges, particularly considering [tools like RunKit endpoints and Jupyter Kernel Gateway](#2259 (comment)), not to mention all the other ways cloud functions can be deployed these days. Ref #1752 #2259

paulmelnikow · 2018-12-07T01:16:26Z

Closing in favor of #2473.

@bkdotcom

This reimplements the idea @bkdotcom came up with in #1519, and took a stab at in #1525. It’s a really powerful way to add all sorts of custom badges, particularly considering [tools like RunKit endpoints and Jupyter Kernel Gateway](#2259 (comment)), not to mention all the other ways cloud functions can be deployed these days.

remove unused var

e4633d3

additional unit tests, testing colorB

3356050

embettered badge-json documentation

b9f0948

added notes to "dynamic" badge (what the params are)

paulmelnikow reviewed Mar 4, 2018

View reviewed changes

paulmelnikow added service-badge Accepted and actionable changes, features, and bugs core Server, BaseService, GitHub auth labels Mar 4, 2018

paulmelnikow mentioned this pull request Mar 4, 2018

Add alternative static badge #1395

Closed

paulmelnikow mentioned this pull request Mar 4, 2018

semantic colorschemes & color keywords #1522

Closed

RedSparr0w mentioned this pull request Jun 27, 2018

[badge/dynamic/json] User defined JSON source badge #820

Merged

mwasilew mentioned this pull request Jun 27, 2018

api: add 'badges' endpoint to project object Linaro/squad#291

Merged

RedSparr0w mentioned this pull request Jun 27, 2018

Add functionality for change color dynamically #1752

Closed

paulmelnikow mentioned this pull request Aug 19, 2018

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

Closed

paulmelnikow added on-hold Deferred in favor of another approach, blocked on preceding efforts, stale, or abandoned needs-discussion A consensus is needed to move forward labels Nov 15, 2018

paulmelnikow mentioned this pull request Nov 17, 2018

replace calls to makeBadge() with BadgeFactory.create() #2336

Closed

paulmelnikow mentioned this pull request Dec 3, 2018

dependency version badges for applications, from package.json #2259

Closed

paulmelnikow mentioned this pull request Dec 7, 2018

[Endpoint] badge #2473

Merged

10 tasks

paulmelnikow closed this Dec 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[badge-json] new "badge-json" service #1525

[badge-json] new "badge-json" service #1525

bkdotcom commented Feb 25, 2018 •

edited by RedSparr0w

Loading

shields-ci commented Feb 25, 2018 •

edited

Loading

bkdotcom commented Feb 25, 2018

platan commented Feb 25, 2018

bkdotcom commented Feb 25, 2018 •

edited

Loading

platan commented Feb 25, 2018 •

edited

Loading

RedSparr0w commented Feb 26, 2018

bkdotcom commented Feb 28, 2018 •

edited

Loading

paulmelnikow commented Mar 4, 2018

paulmelnikow commented Mar 4, 2018

paulmelnikow commented Mar 4, 2018

paulmelnikow left a comment

paulmelnikow Mar 4, 2018

paulmelnikow Mar 4, 2018

RedSparr0w Mar 5, 2018

paulmelnikow Mar 5, 2018

paulmelnikow Mar 4, 2018

paulmelnikow commented Mar 4, 2018

bkdotcom commented Mar 5, 2018 •

edited

Loading

paulmelnikow commented Mar 6, 2018

paulmelnikow commented Mar 21, 2018

paulmelnikow commented Nov 15, 2018

paulmelnikow commented Dec 5, 2018

paulmelnikow commented Dec 7, 2018

[badge-json] new "badge-json" service #1525

[badge-json] new "badge-json" service #1525

Conversation

bkdotcom commented Feb 25, 2018 • edited by RedSparr0w Loading

shields-ci commented Feb 25, 2018 • edited Loading

bkdotcom commented Feb 25, 2018

platan commented Feb 25, 2018

bkdotcom commented Feb 25, 2018 • edited Loading

platan commented Feb 25, 2018 • edited Loading

RedSparr0w commented Feb 26, 2018

bkdotcom commented Feb 28, 2018 • edited Loading

paulmelnikow commented Mar 4, 2018

paulmelnikow commented Mar 4, 2018

paulmelnikow commented Mar 4, 2018

paulmelnikow left a comment

Choose a reason for hiding this comment

paulmelnikow Mar 4, 2018

Choose a reason for hiding this comment

paulmelnikow Mar 4, 2018

Choose a reason for hiding this comment

RedSparr0w Mar 5, 2018

Choose a reason for hiding this comment

paulmelnikow Mar 5, 2018

Choose a reason for hiding this comment

paulmelnikow Mar 4, 2018

Choose a reason for hiding this comment

paulmelnikow commented Mar 4, 2018

bkdotcom commented Mar 5, 2018 • edited Loading

paulmelnikow commented Mar 6, 2018

paulmelnikow commented Mar 21, 2018

paulmelnikow commented Nov 15, 2018

paulmelnikow commented Dec 5, 2018

paulmelnikow commented Dec 7, 2018

bkdotcom commented Feb 25, 2018 •

edited by RedSparr0w

Loading

shields-ci commented Feb 25, 2018 •

edited

Loading

bkdotcom commented Feb 25, 2018 •

edited

Loading

platan commented Feb 25, 2018 •

edited

Loading

bkdotcom commented Feb 28, 2018 •

edited

Loading

bkdotcom commented Mar 5, 2018 •

edited

Loading