[Github] File Size #745

webcaetano · 2016-07-18T05:04:11Z

New badge with specific Github file size information.

Related issue : #730

webcaetano

LMK if you guys need any changes

olivierlacan · 2017-04-08T01:06:36Z

So this is displaying the git repo size? This could be confused with some binary package size. Maybe this would be more appropriate?

I used lightgrey here because I don't really see why the size should be displayed in green. The size isn't positive or negative, it's just informational.

webcaetano · 2017-04-08T14:05:48Z

@olivierlacan hi,
Not repo size. File size.
For client-side libraries ( lodash , underscore , phaser , etc ...) showing their compiled main .min file
Its easier to lib display if it is lightweight on readme.md

paulmelnikow · 2017-04-28T20:11:31Z

Hi, I'd like to get this merged. Would you be willing to add tests using the project's new testing framework?

paulmelnikow · 2017-04-28T20:10:34Z

server.js

+  var customHeaders = {
+    'User-Agent': 'Shields.io',
+    'Accept': 'application/vnd.github.drax-preview+json'
+  };


This should use githubAuth.request, which should take care of most of these issues for you.

Yes, got it.

paulmelnikow · 2017-04-28T20:10:55Z

server.js

+  var path = match[3];
+  var format = match[4];
+
+  var apiUrl = 'https://github.com/gitapi/repos/' + user + '/' + repo + '/contents/' + path;


Would a HEAD request be sufficient to get the size?

Add file size test

webcaetano · 2017-04-29T15:49:04Z

@paulmelnikow done
Awesome job on this new testing setup. 👍
I might create a new PR populating this new github service-test

paulmelnikow · 2017-04-29T18:19:20Z

Thanks, I'm glad to hear that!

Testing more of the github services would be great! Feel free to open an issue to coordinate, if you'd like. Another open PR (#756) has tests of the downloads endpoint, though the rest of the badges are wide open. There is an issue that is causing github to be flaky in CI, which will also need to be fixed at some point (observed here, see this log).

Will leave you a few minor comments.

paulmelnikow · 2017-04-29T18:00:40Z

server.js

+  if (serverSecrets) {
+    apiUrl += '?client_id=' + serverSecrets.gh_client_id
+      + '&client_secret=' + serverSecrets.gh_client_secret;
+  }


You can skip this block too. ^^ The auth should be covered by githubAuth.request.

(Looks like this was copied from the github license integration, which ought to be fixed at some point…)

Could you pick that commit into this branch?

paulmelnikow · 2017-04-29T18:05:32Z

service-tests/github.js

+  .expectJSONTypes(Joi.object().keys({
+    name: Joi.equal('size'),
+    value: Joi.string().regex(/^[0-9]*[.]?[0-9]+\s(B|kB|MB|GB|TB|PB|EB|ZB|YB)$/)
+  }));


❤️

Is it possible to hit both the repo or file not found and unknown file branches? Could you test these as well?

Sorry, I should have explained that better!

The purpose of adding tests is to make sure the repo or file not found code branch and the unknown file code branch are working as intended. There needs to be a separate test for each one, with an exact match on the string in the response. Otherwise we don't know the API can ever return something that will trigger that branch.

For example, you could add a test for /size/some-nonexistent-user/some-nonexistent-repo/some-nonexistent-file.md. I imagine that will hit one of those. Then identify another badge URI which will hit the other.

paulmelnikow · 2017-04-29T18:08:39Z

server.js

+        return;
+      }
+      var body = JSON.parse(buffer);
+      if (body.size != null) {


Nit: This test accepts some unexpected values. Does a json null signify something in the API? If so, the reliable test would be body.size !== null. If not, it would be Number.isInteger(body.size).

Add file size test

webcaetano · 2017-04-29T20:53:42Z

Moved to #976

paulmelnikow · 2017-04-30T01:10:25Z

Thanks for jumping on those tests!

It would be easier to me to finish up the review I started here. Small and atomic PRs are easier, and plus we've already begun the conversation. Hopefully we can merge this in the next day or two, and then proceed with the rest of the tests in #976.

webcaetano · 2017-04-30T02:19:10Z

@paulmelnikow ok i'll organize this

webcaetano · 2017-04-30T02:37:22Z

@paulmelnikow this is done.
Atomic PRs make sense. Make things more splited and organized.
I'll reorganize #976 now

paulmelnikow · 2017-04-30T19:18:42Z

service-tests/github.js

+  .expectJSONTypes(Joi.object().keys({
+    name: Joi.equal('size'),
+    value: Joi.string().regex(/^unknown file$/),
+  }));


It looks like this is what happens when you point the badge at e.g. a directory.

Would "not a regular file" be a more helpful message?

@paulmelnikow sure.

Hey, sorry, I meant in the output as well: "unknown file" -> "not a regular file".

@paulmelnikow ok

Sorry, I'm still not being clear enough.

Could you set badgeData.text[1] = 'not a regular file' so the user sees that message on the badge?

@paulmelnikow 58f2e6f done
Be welcome to make commits to my PR

paulmelnikow · 2017-04-30T19:19:18Z

service-tests/github.js

+    value: Joi.string().regex(/^unknown file$/),
+  }));
+
+t.create('downloads for release without slash')


Could you remove these download tests?

@paulmelnikow this is from the #756 PR.
I Fork from there. He was the first to create the file.
I fixed potential conflicts.
What should we do?

Ah, you can just remove the lines and commit the change.

Everything will get squashed, so the individual commits will go away. Probably this will go in first, and then #756 will need to solve a minor merge conflict (a new file created in both branches).

@paulmelnikow ok

paulmelnikow · 2017-04-30T19:20:14Z

server.js

@@ -3113,7 +3114,7 @@ cache(function(data, match, sendBadge, request) {
 }));

 // GitHub release-download-count integration.
-camp.route(/^\/github\/downloads\/([^\/]+)\/([^\/]+)(\/[^\/]+)?\/([^\/]+)\.(svg|png|gif|jpg|json)$/,
+camp.route(/^\/github\/downloads\/([^\/]+)\/([^\/]+)(\/.+)?\/([^\/]+)\.(svg|png|gif|jpg|json)$/,


Could you reset this change? ^^

@paulmelnikow also from the #756 pr

webcaetano · 2017-04-30T19:47:02Z

@paulmelnikow done
Lmk if you need any changes.
I'll keep my eyes on the #976

webcaetano · 2017-05-01T16:50:24Z

After merge this we can close #730

webcaetano · 2017-05-01T18:17:02Z

I have no clue what's this travis error right now

paulmelnikow · 2017-05-01T18:27:35Z

I don't see the error.

Thanks for the changes! Merging.

webcaetano · 2017-05-01T18:34:57Z

@paulmelnikow #979 that is what happen

espadrine · 2017-08-30T19:56:42Z

@webcaetano @paulmelnikow I get an error on this in npm run test:services. Do you have a clue what happens by any chance? @Daniel15 We can't enable that to be part of npm test unless all of it passes.

1) Github File size 
        [ GET http://localhost:1111/github/size/webcaetano/craft/build/craft.min.js.json ]:
     ValidationError: child "value" fails because ["value" with value "repo or file not found" fails to match the required pattern: /^[0-9]*[.]?[0-9]+\s(B|kB|MB|GB|TB|PB|EB|ZB|YB)$/]
      at Object.exports.process (node_modules/joi/lib/errors.js:181:19)
      at Object._validateWithOptions (node_modules/joi/lib/types/any/index.js:651:31)
      at Object.root.validate (node_modules/joi/lib/index.js:121:23)
      at Object.pathMatch.matchJSONTypes (node_modules/icedfrisby/lib/pathMatch.js:299:9)
      at node_modules/icedfrisby/lib/icedfrisby.js:721:10
      at IcedFrisbyNock._invokeExpects (node_modules/icedfrisby/lib/icedfrisby.js:1200:33)
      at node_modules/icedfrisby/lib/icedfrisby.js:1179:29
      at Request.runCallback [as _callback] (node_modules/icedfrisby/lib/icedfrisby.js:561:11)
      at Request.self.callback (node_modules/request/request.js:188:22)
      at Request.<anonymous> (node_modules/request/request.js:1171:10)
      at IncomingMessage.<anonymous> (node_modules/request/request.js:1091:12)
      at endReadableNT (_stream_readable.js:975:12)
      at _combinedTickCallback (internal/process/next_tick.js:80:11)
      at process._tickDomainCallback (internal/process/next_tick.js:128:9)

webcaetano · 2017-08-30T20:02:53Z

@espadrine yes i do.
That was my fault. I changed the file name of my repo recently.
@espadrine i'll make a PR fixing that

espadrine · 2017-08-30T20:12:43Z

@webcaetano OK thanks a lot! Don't worry, there's no rush, I won't look at it until next Saturday (but I will definitely look at it on Saturday if the PR is ready ☺)

webcaetano · 2017-08-30T21:15:12Z

@espadrine #1066

yairEO · 2018-09-19T09:25:25Z

Can someone explain to me how to use this size shield?
I'm extremely confused.

The link here generates a hard-coded size and I was thinking I somewhere need to input my repo's url or maybe the badge's server is smart enough to know the repo's URL from the server that requests the image and construct a file size (but then again.. what does it really measures...which file size)

I'm stuck on how am I supposed to use this badge and find zero information on how should this badge be used in the docs. simply dumping it there only shows a hard-coded value that I do not see a way to change

paulmelnikow · 2018-09-19T18:21:29Z

Hi, thanks for your question. Sorry this isn't clear.

If you go to https://shields.io/#/examples/size and click on the badge you'll see a page for the badge, which shows an example badge URL:

https://img.shields.io/github/size/webcaetano/craft/build/phaser-craft.min.js.svg

webcaetano/craft is the repo and build/phaser-craft.min.js is the filename. Hope that helps!

github file size

b5ead82

webcaetano mentioned this pull request Jul 19, 2016

Add file size badges #730

Closed

Support GitHub releases containing a / in the tag

2e4c86d

espadrine force-pushed the master branch from b5e14ce to b74107e Compare September 8, 2016 23:24

webcaetano commented Sep 15, 2016

View reviewed changes

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

paulmelnikow reviewed Apr 28, 2017

View reviewed changes

NHellFire added 2 commits April 28, 2017 21:45

Merge remote-tracking branch 'upstream/master'

fb337bc

[GitHub] Add service test for download counts

f8432b3

webcaetano force-pushed the file-size branch from 20e806d to b5ead82 Compare April 29, 2017 12:49

webcaetano added 2 commits April 29, 2017 10:24

Merged master into badges#745 | Resolved conflicts

74bb890

badges#745 : use githubAuth.request

dc0983f

webcaetano changed the title ~~[New badge] Github file size~~ Add New badge Github file size Apr 29, 2017

webcaetano changed the title ~~Add New badge Github file size~~ [GithubFileSize] Github file size Apr 29, 2017

webcaetano changed the title ~~[GithubFileSize] Github file size~~ [githubFileSize] Github file size Apr 29, 2017

webcaetano changed the title ~~[githubFileSize] Github file size~~ [github] Github file size Apr 29, 2017

webcaetano changed the title ~~[github] Github file size~~ [Github] Github file size Apr 29, 2017

webcaetano changed the title ~~[Github] Github file size~~ [Github] File Size Apr 29, 2017

badges#745 | badges#937 : create github service-test

ca67136

Add file size test

paulmelnikow reviewed Apr 29, 2017

View reviewed changes

webcaetano mentioned this pull request Apr 29, 2017

[Github] Populate Service Test #976

Closed

webcaetano added a commit to webcaetano/shields that referenced this pull request Apr 29, 2017

badges#745 : use githubAuth.request

3405f37

webcaetano added a commit to webcaetano/shields that referenced this pull request Apr 29, 2017

badges#745 | badges#937 : create github service-test

3ed09b1

Add file size test

webcaetano closed this Apr 29, 2017

webcaetano added 3 commits April 29, 2017 23:24

Merge remote-tracking branch 'nh/master' into file-size

c6b7158

[Github] File Size: improvements

b79c3e9

[Github] test: hold file not found | and file unknow

f3bdca1

paulmelnikow mentioned this pull request Apr 30, 2017

Github rate limits cause transient service test failures in CI #979

Closed

paulmelnikow reviewed Apr 30, 2017

View reviewed changes

webcaetano added 3 commits April 30, 2017 16:29

[Github] downloads: rollback camp route

5042041

[Github] test: file size: change label for unknow file

a825ca0

[Github] remove donwloads

254fa3f

[Github] test: File size: change label unknow file

70b0ba9

webcaetano force-pushed the file-size branch from e9bf707 to 58f2e6f Compare May 1, 2017 17:54

[Github] Filesize: changes

2225b41

webcaetano force-pushed the file-size branch from 58f2e6f to 2225b41 Compare May 1, 2017 18:20

paulmelnikow merged commit a6d81f2 into badges:master May 1, 2017

paulmelnikow added this to the Next Deploy milestone May 1, 2017

chris48s mentioned this pull request Sep 4, 2024

Add ability to format bytes as metric (e.g: kilobyte) or IEC (e.g: kibibyte) format #10437

Open

[Github] File Size #745

[Github] File Size #745

Conversation

webcaetano commented Jul 18, 2016

webcaetano left a comment

Choose a reason for hiding this comment

olivierlacan commented Apr 8, 2017

webcaetano commented Apr 8, 2017 • edited Loading

paulmelnikow commented Apr 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webcaetano commented Apr 29, 2017 • edited Loading

paulmelnikow commented Apr 29, 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

webcaetano commented Apr 29, 2017

paulmelnikow commented Apr 30, 2017

webcaetano commented Apr 30, 2017

webcaetano commented Apr 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

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

webcaetano commented Apr 30, 2017

webcaetano commented May 1, 2017

webcaetano commented May 1, 2017

paulmelnikow commented May 1, 2017

webcaetano commented May 1, 2017

espadrine commented Aug 30, 2017

webcaetano commented Aug 30, 2017

espadrine commented Aug 30, 2017

webcaetano commented Aug 30, 2017

yairEO commented Sep 19, 2018 • edited Loading

paulmelnikow commented Sep 19, 2018

webcaetano commented Apr 8, 2017 •

edited

Loading

webcaetano commented Apr 29, 2017 •

edited

Loading

yairEO commented Sep 19, 2018 •

edited

Loading