[ready for review] [new badge] added Sourcegraph.com badge #828

rohanpai · 2016-11-04T18:25:11Z

Sourcegraph is how developers discover and understand code. As part of this, we can answer questions like "How many repositories use my library?" easily. We'd like to propose this as a badge:

This badge lets anyone with a Go repository list how many people are actually using their library in their repository. (languages other than Go are coming soon)

To try this badge out locally:

PORT=8083 node server
http://localhost:8083/sourcegraph/rrc/github.com/gorilla/mux.svg

rohanpai · 2017-01-12T19:22:33Z

give it a shot! http://localhost/sourcegraph/github.com/gorilla/mux.svg

rohanpai · 2017-01-12T19:32:59Z

Could I get a review ? @PeterDaveHello @espadrine Thanks in advance!

PeterDaveHello · 2017-01-12T19:52:05Z

Hi @rohanpai, actually I'm not an active contributor or even maintainer here, I think you'll need somebody else to approve your pull request 😄

espadrine

Thanks a lot for the patch!

Could you squash your commits together please?

espadrine · 2017-01-14T14:01:17Z

README.md

@@ -84,6 +84,7 @@ What kind of metadata can you convey using badges?
 * [ruby-gem-downloads-badge](https://github.com/bogdanRada/ruby-gem-downloads-badge/)
 * [Scrutinizer](https://scrutinizer-ci.com/)
 * [Semaphore](https://semaphoreapp.com)
+* [Sourcegraph](https://sourcegraph.com)


This is not a list of integrations that the website supports. It is a list of third-parties that attempt to have an implementation conforming to the standard look. I believe that Sourcegraph does not have its own implementation of badges. If that is correct, could you remove this line?

espadrine · 2017-01-14T14:38:08Z

index.html

@@ -232,6 +232,10 @@ <h3 id="build"> Build </h3>
 </tbody></table>
 <h3 id="downloads"> Downloads </h3>
 <table class='badge'><tbody>
+  <tr><th data-keywords='sourcegraph' data-doc='sourcegraphDoc'> Sourcegraph (# references other repos): </th>


It looks like you are not using documentation, so data-doc='sourcegraphDoc does not reference a DOM object with id="sourcegraphDoc". Additionally, you do not need to put data-keywords='sourcegraph', since that word is already in the text of the th.

Also, this badge is unrelated to downloads. Could you put it at the bottom of the miscellaneous section, please?

Finally, perhaps you can simplify the title to make it shorter? What matters most is that people can understand what it does; hopefully the badge is enough, so that the part in parentheses is not necessary.

espadrine · 2017-01-14T14:38:33Z

logo/sourcegraph.svg

+<path fill="#FFFFFF" d="M69.087,56.734c-0.798,0-1.597-0.2-2.596-0.399l-60.9-19.968C1.398,34.97-0.998,30.377,0.4,26.184
+	c1.397-4.193,5.99-6.589,10.183-5.191l60.9,19.967c4.193,1.397,6.59,5.99,5.191,10.184C75.677,54.538,72.682,56.734,69.087,56.734z"
+	/>
+</svg>


Could you pass this file through SVGo to make it shorter?

espadrine · 2017-01-14T14:38:50Z

server.js

@@ -2989,6 +2989,39 @@ cache(function(data, match, sendBadge, request) {
  });
 }));

+camp.route(/^\/sourcegraph\/([\s\S]+)\.(svg|png|gif|jpg|json)$/,
+cache(function (data, match, sendBadge, request) {
+


Could you remove this empty line please?

espadrine · 2017-01-14T14:40:51Z

server.js

@@ -2989,6 +2989,39 @@ cache(function(data, match, sendBadge, request) {
  });
 }));

+camp.route(/^\/sourcegraph\/([\s\S]+)\.(svg|png|gif|jpg|json)$/,


We wish to allow for vendor URLs to support extending it as vendors support more pieces of information. Could you add a part related to this particular piece of information offered by Sourcegraph? Perhaps /sourcegraph/rrc/…, for "repository reference count"?

espadrine · 2017-01-14T14:52:03Z

server.js

+  var repo = match[1];  // eg, `prismic` or `foo/bar`.
+  var format = match[2];
+  var apiUrl = "https://sourcegraph.com/.api/repos/"+repo+"/-/shield"
+  var badgeData = getBadgeData('build', data);


Are you sure that "build" is the best left-hand-side description of what the badge does? I feel like "dependents" or "used by" would be more appropriate, if I understand this badge correctly.

espadrine · 2017-01-14T14:52:35Z

server.js

+  var badgeData = getBadgeData('build', data);
+  request(apiUrl, function(err, res, buffer) {
+      console.log("err" + err)
+    if (err !== null) {


err != null, to also work if err is undefined.

espadrine · 2017-01-14T14:52:55Z

server.js

+    }
+    badgeData.text[0] = "Used by"
+    badgeData.colorscheme = 'brightgreen';
+    badgeData.logo = logos['sourcegraph'];


logos.sourcegraph

espadrine · 2017-01-14T14:53:30Z

server.js

+      sendBadge(format, badgeData);
+      return;
+    }
+    badgeData.text[0] = "Used by"


Could you start the try {…} before this line?

Also, you can remove that line, since it is already taken care of by line 2998.

(Also, the left-hand-side almost always starts with a lowercase.)

espadrine · 2017-01-14T14:55:15Z

try.html

+  <tr><th data-keywords='sourcegraph' data-doc='sourcegraphDoc'> Sourcegraph (# references other repos): </th>
+  <td><img src='/sourcegraph/github.com/gorilla/mux.svg' alt=''/></td>
+  <td><code>https://img.shields.io/sourcegraph/github.com/gorilla/mux.svg</code></td>
+  </tr>


Same as the comments in index.html. By the way, you should remove the changes you made in index.html; I generate that file automatically, as hinted at the top of index.html.

rohanpai · 2017-01-15T02:14:36Z

@espadrine let me know what you think!

RedSparr0w · 2017-01-15T02:23:21Z

server.js

+      return;
+    }
+    try {
+      badgeData.text[0] = "Used by";


Think this line still needs to be removed and change Line 2997 to var badgeData = getBadgeData('used by', data);

or leave Line 2997 as is, but badgeData.text[0] is set via the first parameter when you call getBadgeData();

rohanpai · 2017-01-16T09:57:36Z

Made requested changes cc @RedSparr0w @espadrine

Thank you!

rohanpai · 2017-01-18T20:30:25Z

@espadrine does this look ready to merge? Thanks!

rohanpai · 2017-01-31T00:08:06Z

Hi @RedSparr0w @espadrine ! Any update on this? We've got a bunch of users who are really excited about using this :)

All feedback has been addressed & I've updated the PR description.

RedSparr0w · 2017-01-31T20:42:27Z

@rohanpai Hey, sorry i don't actually have any permissions in this repo, just thought i should mention the change so it would have a better chance of getting merged sooner than later.
Although i'm sure it wont be too long before it gets merged. [=

Daniel15 · 2017-05-06T04:30:34Z

This looks pretty good to me! Sorry for the delay in getting back to you.

As a followup, could you please add an automated test for this badge? You can find the documentation to do so here: https://github.com/badges/shields/blob/master/service-tests/README.md

Thank you for your contribution!

rohanpai closed this Nov 4, 2016

rohanpai reopened this Nov 4, 2016

rohanpai changed the title ~~[new badge] added Sourcegraph.com badge~~ [wip] [new badge] added Sourcegraph.com badge Nov 4, 2016

rohanpai force-pushed the master branch from 2cd7ca5 to fe7b0a2 Compare January 12, 2017 18:54

rohanpai changed the title ~~[wip] [new badge] added Sourcegraph.com badge~~ [ready for review] [new badge] added Sourcegraph.com badge Jan 12, 2017

espadrine requested changes Jan 14, 2017

View reviewed changes

rohanpai force-pushed the master branch 3 times, most recently from aff0c34 to 2281c3d Compare January 15, 2017 02:13

RedSparr0w reviewed Jan 15, 2017

View reviewed changes

rohanpai force-pushed the master branch from 2281c3d to cfe790b Compare January 16, 2017 09:53

add Sourcegraph.com badge

308dde7

slimsag force-pushed the master branch from cfe790b to 308dde7 Compare January 31, 2017 00:05

paulmelnikow added the service-badge Accepted and actionable changes, features, and bugs label Mar 27, 2017

Daniel15 merged commit f97d5a6 into badges:master May 6, 2017

Daniel15 added this to the Next Deploy milestone May 6, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ready for review] [new badge] added Sourcegraph.com badge #828

[ready for review] [new badge] added Sourcegraph.com badge #828

rohanpai commented Nov 4, 2016 •

edited

rohanpai commented Jan 12, 2017

rohanpai commented Jan 12, 2017

PeterDaveHello commented Jan 12, 2017

espadrine left a comment

espadrine Jan 14, 2017

espadrine Jan 14, 2017

espadrine Jan 14, 2017

espadrine Jan 14, 2017

espadrine Jan 14, 2017

espadrine Jan 14, 2017

espadrine Jan 14, 2017

espadrine Jan 14, 2017

espadrine Jan 14, 2017

espadrine Jan 14, 2017

espadrine Jan 14, 2017

rohanpai commented Jan 15, 2017

RedSparr0w Jan 15, 2017 •

edited

rohanpai commented Jan 16, 2017

rohanpai commented Jan 18, 2017

rohanpai commented Jan 31, 2017

RedSparr0w commented Jan 31, 2017

Daniel15 commented May 6, 2017

[ready for review] [new badge] added Sourcegraph.com badge #828

[ready for review] [new badge] added Sourcegraph.com badge #828

Conversation

rohanpai commented Nov 4, 2016 • edited

rohanpai commented Jan 12, 2017

rohanpai commented Jan 12, 2017

PeterDaveHello commented Jan 12, 2017

espadrine 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

rohanpai commented Jan 15, 2017

RedSparr0w Jan 15, 2017 • edited

Choose a reason for hiding this comment

rohanpai commented Jan 16, 2017

rohanpai commented Jan 18, 2017

rohanpai commented Jan 31, 2017

RedSparr0w commented Jan 31, 2017

Daniel15 commented May 6, 2017

rohanpai commented Nov 4, 2016 •

edited

RedSparr0w Jan 15, 2017 •

edited