Github: Support PR/issues with labels, full count #1020

dagwieers · 2017-06-27T16:16:28Z

This PR includes the following:

Simplified the code, but kept it full-featured
- Use the issues API for all calls to Github
- Got rid of the repos API for some of the queries
It now displays more exact issue numbers (no longer indicates 30+)
It can now display the number of PRs by label too (not just issues only)
Can display the used label name too for completeness

This fixes #839
This fixes #420
This implements #862 partly

jhawkesworth · 2017-06-27T20:37:20Z

nice!

StoneCypher · 2017-06-28T02:21:47Z

can the issues by-label include the label though? without that it might quickly get confusing

dagwieers · 2017-06-28T07:44:20Z

@StoneCypher In our use-case, the context is very clear, so there is no need for that.
e.g. https://github.com/ansible/community/wiki/Windows:-action-list

But if you're going to add multiple of these on a single page, I see your point.
The question is how we are going to tell the code when the label should be shown.

Let me know what you like here. We could add -label to in there somewhere ?

dagwieers · 2017-06-28T14:26:03Z

@StoneCypher I now do this:

So basically, if you provide -raw it does not add the label, otherwise it will.
Is this OK for you ?

StoneCypher · 2017-06-28T15:52:37Z

ya 😄

dagwieers · 2017-06-28T15:56:44Z

@StoneCypher Enjoy !

PS Now I have to find out how to get this merged, because I would like to use it for real !

StoneCypher · 2017-06-28T16:12:07Z

Well, if I tag @espadrine , then you weren't the obnoxious one 😀

dagwieers · 2017-07-04T23:49:35Z

@StoneCypher Not sure if you had any summoning powers before, but they are surely failing now ;-)

StoneCypher · 2017-07-05T00:31:45Z

they did indeed

StoneCypher · 2017-09-11T18:29:10Z

@Daniel15 - any chance of looking at this? This is rly valuable

paulmelnikow

Hi! Thanks for this. Could you add a service test which covers the new functionality?

dagwieers · 2017-09-23T08:03:51Z

@paulmelnikow Done.

The original code has been simplified as well, but commented out. I don't think we need the original code using the issues API, as it has some limitations (no label support, only up to 30, or 100 issues).

As requested, we now add the GitHub label to the badge, only if not specifying `-raw` in the URL.

StoneCypher · 2017-09-23T15:22:59Z

@paulmelnikow (thank you)

paulmelnikow · 2017-09-24T01:37:18Z

Am I correct that /github/issues now returns only issues, and not issues + PRs?

paulmelnikow

Tests look great! Clarifies the API too, which I've made some comments on. Really excited about this functionality. Shields could use a count of beginner-friendly issues on the readme or contributing pages.

paulmelnikow · 2017-09-24T01:40:00Z

service-tests/github.js

+  .expectJSONTypes(Joi.object().keys({
+    name: Joi.equal('issues'),
+    value: Joi.string().regex(/^[0-9]+[kMGTPEZY]?$/)
+  }));


What do you think about calling this -minimalist or -clean, or better yet, /issues/minimalist, rather than -raw? It seems to clarify that it's affecting the formatting, rather than the underlying number.

I didn't introduce -raw, it was called like this before. I don't think we an break the existing interface.

paulmelnikow · 2017-09-24T01:41:21Z

service-tests/github.js

+  .expectJSONTypes(Joi.object().keys({
+    name: Joi.equal('closed issues'),
+    value: Joi.string().regex(/^[0-9]+[kMGTPEZY]? closed$/)
+  }));


It seems to me that closed is not needed here, since the left side already says it's counting closed issues. Then we could omit -closed-raw as well.

Rights, I think this was done because it's closer to what we do for open. Otherwise there's no difference anymore between closed-raw and closed. It's not simple to omit something, except maybe from the listing, but people can still do it.

Up to you to decide. I'll do whatever makes it upstream ;-) My interest in displaying closed issues or PRs is non-existing.

almost my entire interest here is in closed issues (though i do not understand or care about the -raw) distinction

Haha. Ok, I'll do what you suggested.

paulmelnikow · 2017-09-24T01:44:30Z

service-tests/github.js

+  .expectJSONTypes(Joi.object().keys({
+    name: Joi.equal('pull requests'),
+    value: Joi.string().regex(/^[0-9]+[kMGTPEZY]? open$/)
+  }));


Should this just be /pull-requests instead of /issues-pr?

Ah, we need to support /issues-pr for backward compatibility. What do you think about supporting both?

Yup, so I only simplified the implementation and made the numbers exact (rather than saying 30+ for everything larger than 30). And that's what we needed. The simplification was simple a result of the fact that it was needed for getting proper numbers.

dagwieers · 2017-09-24T18:50:01Z

Am I correct that /github/issues now returns only issues, and not issues + PRs?

That is exactly as it was before AFAIK. You either have issues or PRs (and I don't know what the name is for both).

dagwieers · 2017-09-24T19:24:43Z

Done !

server.js

dagwieers · 2017-09-24T19:58:04Z

Latest result:

dagwieers · 2017-09-24T19:59:15Z

Ready to be merged ! (And then I can update our community wiki pages to get real numbers !)

This PR includes: - Getting rid of the option that includes 2x closed - Using -closed-raw is no longer useful

espadrine · 2017-09-24T20:15:33Z

Merged and live.

dagwieers · 2017-09-24T21:05:01Z

@espadrine Thanks ! I was making some more small changes to the examples, but I'll add them to another PR. Nothing important.

StoneCypher · 2017-09-24T21:39:36Z

Working in production. This used to say issues 30+ closed 😀

paulmelnikow · 2017-09-29T19:53:44Z

@dagwieers It's really cool to see this in use!

https://github.com/ansible/community/wiki/Ambassadors

AraHaan · 2021-02-11T21:18:26Z

@espadrine what about a badge for merged PR's only too?

calebcartwright · 2021-02-12T01:27:25Z

@AraHaan - thank you for looking back through the history to find things relevant to your inquiry, but a pull request that was closed more than 3 years isn't the best place for a new feature request. Please open a new issue if you'd like a new badge/new variant of a badge to increase the odds of someone potentially implementing it!

AraHaan · 2021-02-12T01:55:09Z

ok opened #6164

dagwieers force-pushed the github-fullsupport branch 10 times, most recently from 2a990a1 to 6fdb4c7 Compare June 27, 2017 17:21

dagwieers force-pushed the github-fullsupport branch 4 times, most recently from 168c081 to 3823df9 Compare June 27, 2017 22:53

paulmelnikow assigned paulmelnikow and unassigned paulmelnikow Sep 22, 2017

paulmelnikow requested changes Sep 22, 2017

View reviewed changes

dagwieers added 2 commits September 23, 2017 10:04

Github: Support PR/issues with labels, full count

e0ecadd

The original code has been simplified as well, but commented out. I don't think we need the original code using the issues API, as it has some limitations (no label support, only up to 30, or 100 issues).

Add the label to the badge when not using -raw

22694e6

As requested, we now add the GitHub label to the badge, only if not specifying `-raw` in the URL.

dagwieers force-pushed the github-fullsupport branch from b29967c to 22694e6 Compare September 23, 2017 08:05

dagwieers mentioned this pull request Sep 23, 2017

[Github] Optionally add name to release badge #1021

Closed

paulmelnikow self-assigned this Sep 24, 2017

paulmelnikow reviewed Sep 24, 2017

View reviewed changes

espadrine reviewed Sep 24, 2017

View reviewed changes

server.js Outdated Show resolved Hide resolved

dagwieers force-pushed the github-fullsupport branch 3 times, most recently from 7c336ca to 57f88f7 Compare September 24, 2017 19:46

dagwieers force-pushed the github-fullsupport branch from 57f88f7 to af4b1ad Compare September 24, 2017 20:06

Get rid of repeated "closed"

62c470d

This PR includes: - Getting rid of the option that includes 2x closed - Using -closed-raw is no longer useful

dagwieers force-pushed the github-fullsupport branch from af4b1ad to 62c470d Compare September 24, 2017 20:09

espadrine closed this Sep 24, 2017

This was referenced Sep 26, 2017

[GitHub] Fixes to service tests and try.html #1091

Closed

Should GitHub issue badge including pull requests? #420

Closed

Add GitHub issue-nonpr badge #839

Closed

dagwieers mentioned this pull request Sep 28, 2017

GitHub badges for all communities ansible/community#258

Closed

paulmelnikow mentioned this pull request Oct 4, 2017

Github open issues with specific tag #862

Closed

Github: Support PR/issues with labels, full count #1020

Github: Support PR/issues with labels, full count #1020

Conversation

dagwieers commented Jun 27, 2017 • edited Loading

jhawkesworth commented Jun 27, 2017

StoneCypher commented Jun 28, 2017

dagwieers commented Jun 28, 2017

dagwieers commented Jun 28, 2017

StoneCypher commented Jun 28, 2017

dagwieers commented Jun 28, 2017

StoneCypher commented Jun 28, 2017

dagwieers commented Jul 4, 2017

StoneCypher commented Jul 5, 2017

StoneCypher commented Sep 11, 2017

paulmelnikow left a comment

Choose a reason for hiding this comment

dagwieers commented Sep 23, 2017 • edited Loading

StoneCypher commented Sep 23, 2017

paulmelnikow commented Sep 24, 2017

paulmelnikow left a comment

Choose a reason for hiding this comment

paulmelnikow Sep 24, 2017

Choose a reason for hiding this comment

dagwieers Sep 24, 2017 • edited Loading

Choose a reason for hiding this comment

paulmelnikow Sep 24, 2017

Choose a reason for hiding this comment

dagwieers Sep 24, 2017 • edited Loading

Choose a reason for hiding this comment

StoneCypher Sep 24, 2017

Choose a reason for hiding this comment

dagwieers Sep 24, 2017

Choose a reason for hiding this comment

paulmelnikow Sep 24, 2017

Choose a reason for hiding this comment

paulmelnikow Sep 24, 2017

Choose a reason for hiding this comment

dagwieers Sep 24, 2017

Choose a reason for hiding this comment

dagwieers commented Sep 24, 2017

dagwieers commented Sep 24, 2017

dagwieers commented Sep 24, 2017 • edited Loading

dagwieers commented Sep 24, 2017 • edited Loading

espadrine commented Sep 24, 2017

dagwieers commented Sep 24, 2017

StoneCypher commented Sep 24, 2017

paulmelnikow commented Sep 29, 2017

AraHaan commented Feb 11, 2021

calebcartwright commented Feb 12, 2021

AraHaan commented Feb 12, 2021

dagwieers commented Jun 27, 2017 •

edited

Loading

dagwieers commented Sep 23, 2017 •

edited

Loading

dagwieers Sep 24, 2017 •

edited

Loading

dagwieers Sep 24, 2017 •

edited

Loading

dagwieers commented Sep 24, 2017 •

edited

Loading

dagwieers commented Sep 24, 2017 •

edited

Loading