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

[GitHubGistStars] add GitHub Gist Stars #8471

Merged
merged 6 commits into from
Oct 4, 2022

Conversation

notanmay
Copy link
Contributor

@notanmay notanmay commented Oct 3, 2022

closes #8269
added a new service class which extends GithubAuthV4Service . It uses Github GraphQL API to query the stargazers count for the provided gist id. also added tester file for the same service class.

screen-shots of example badges generated:
https://gist.github.com/47a4d00457a92aa426dbd48a18776322
sss (1)

https://gist.github.com/invalid-gist-id
Screenshot from 2022-10-03 18-44-40 (1) (1)

@shields-ci
Copy link

shields-ci commented Oct 3, 2022

Messages
📖 ✨ Thanks for your contribution to Shields, @notanmay!

Generated by 🚫 dangerJS against 34b6848

Copy link
Member

@chris48s chris48s 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 picking this up. It is in pretty good shape already. Just a few minor comments.

Comment on lines 12 to 13
stargazerCount: Joi.number(),
url: Joi.string(),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
stargazerCount: Joi.number(),
url: Joi.string(),
stargazerCount: Joi.number().required(),
url: Joi.string().required(),


static route = {
base: 'github/stars/gists',
pattern: ':gist',
Copy link
Member

Choose a reason for hiding this comment

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

Can we make the param :gistId to match

static route = { base: 'github-gist/last-commit', pattern: ':gistId' }

}

static render({ stargazerCount, url }) {
return { message: metric(stargazerCount), link: [url] }
Copy link
Member

Choose a reason for hiding this comment

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

On the repo stars badge, we link the left side of the badge to the repo URL and the right side to /stargazers. Can we do the same thing for the gist version too please.

link: [
`https://github.com/${slug}`,
`https://github.com/${slug}/stargazers`,
],

Gists also have a similar stargazers URL e.g: https://gist.github.com/maratori/47a4d00457a92aa426dbd48a18776322/stargazers

@chris48s chris48s added the service-badge New or updated service badge label Oct 3, 2022
@notanmay
Copy link
Contributor Author

notanmay commented Oct 4, 2022

hi @chris48s , thanks for suggesting these changes. Please review the commit i just pushed. I have committed those changes and updated test cases as well and it passes the tests.

Copy link
Member

@chris48s chris48s left a comment

Choose a reason for hiding this comment

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

Nice. Thanks for working on this.

@repo-ranger repo-ranger bot merged commit 78b5a6b into badges:master Oct 4, 2022
@notanmay
Copy link
Contributor Author

notanmay commented Oct 5, 2022

Thanks for merging my PR!🚀

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.

Badge request: stars for github gist
3 participants