Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GitHub] Top language, languages count and code size badges #1160

Merged
merged 6 commits into from
Oct 12, 2017

Conversation

PyvesB
Copy link
Member

@PyvesB PyvesB commented Oct 11, 2017

This pull request is related to issue #1139 .

In addition to the main programming language badge discussed there, these commits also add two additional badges making use of the same languages API endpoint, but parsing the received data slightly differently.

The resulting badges should look similar to the following:

  • top language top
  • count of languages count
  • code size in bytes bytes

Hope I have followed the guidelines correctly and looking forward to any review comments and feedback! 👍

Cheers,

Pyves

Copy link
Member

@paulmelnikow paulmelnikow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

server.js Outdated
}
badgeData.text[0] = 'code size';
badgeData.text[1] = metric(sumBytes) + 'B';
badgeData.colorscheme = 'blue';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really glad to have a repo size badge! Though, it would be more reliable to get this information from the Repositories endpoint. Would you be up for implementing it that way instead, under a different URL path?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would you get the code size directly using that endpoint? The size parameter seemed to be a good candidate, but must actually relate to something else, as it indicates only 4945 for this repository for instance.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that is right. It's in kilobytes. Here's a StackOverflow answer that talks about it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more I think about it, the repo size is more about how many revisions and binary blobs are checked in. The size of the identifiable code could be useful, distinct from the repo size.

Do they provide lines of code? That would also be really cool. 😃

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think it's the size on disk rather than the code size. I would be inclined to keep the code size badge as it is, but I can also provide a repo size badge through a new pull request, if you want!
They do not provide lines of code directly, but this StackOverflow answer suggests a nice approach to get it. Yet another pull request coming up? 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Yes, a separate badge + PR for repo size makes sense to me.

Wow, if that SO answer works, that would be cool!

Copy link
Member Author

@PyvesB PyvesB Oct 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll take care of that!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the solutions proposed to calculate the lines of code using the API are dodgy according to a comment on the answer I had previously linked. I don't think a line count badge can be implemented reliably and efficiently with the current API possibilities.

.get('/languages/bytes/badges/shields.json')
.expectJSONTypes(Joi.object().keys({
name: Joi.equal('code size'),
value: Joi.string().regex(/^[0-9]*(k|M|G|T|P|E|Z|Y)B$/),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use the isFileSize helper from service-tests/helpers/validators.js.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, I had forked the repository before the helpers were added, this is now fixed.

.get('/languages/count/badges/shields.json')
.expectJSONTypes(Joi.object().keys({
name: Joi.equal('languages'),
value: Joi.string().regex(/^[0-9]*$/),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Joi.number().integer().positive() should work here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Fixed.

.expectJSONTypes(Joi.object().keys({
name: Joi.equal('language'),
value: Joi.equal('none'),
}));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can tighten this up a bit since these are literals:

.expectJSON({ name: 'language', value: 'none' })

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@paulmelnikow paulmelnikow added the service-badge New or updated service badge label Oct 11, 2017
server.js Outdated
sumBytes += parseInt(parsedData[language]);
}
badgeData.text[0] = 'code size';
badgeData.text[1] = metric(sumBytes) + 'B';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you use prettyBytes instead?

server.js Outdated
badgeData.text[1] = Object.keys(parsedData).length;
badgeData.colorscheme = 'blue';
break;
case 'bytes':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about code-size or size to match the label?

@ice1000
Copy link
Contributor

ice1000 commented Oct 12, 2017

Awesome 👍

I'd like to see this available! 😃

@paulmelnikow
Copy link
Member

Github service tests passing locally with the patch from #1118.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service-badge New or updated service badge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants