Add GitLab CI badge support #841
Conversation
Address #541
|
Tested. Additionally, I have implemented in a different way so that it only shows success or failed. |
|
Any plans to rebase and merge this PR? |
|
Hi, thanks for this contribution. I might be missing a little context here. Is there a need for multiple auth tokens? Does a single token run into rate limits? |
|
They haven't implement rate limiting so far. |
|
In that case, you can do this a bit more simply. You can pull the secret directly from |
| var branch = match[3]; | ||
| var format = match[4]; | ||
| var apiUrl = gitlabApiUrl + '/projects/' + encodeURIComponent(userRepo) + '/pipelines'; | ||
| console.log(apiUrl); |
| } | ||
| } | ||
| if(!done) { | ||
| callback(branch, page+1); |
There was a problem hiding this comment.
@espadrine has mentioned that requests need to be bounded in time and complexity.
Could this be modified to make a max of two or three requests?
| } | ||
| }); | ||
| } | ||
| if(branch == null) { |
There was a problem hiding this comment.
To reduce complexity, could the branch default to master unless the user specifies a different one in the badge? Alternatively, the user could be required to specify a branch name.
| var apiUrl = gitlabApiUrl + '/projects/' + encodeURIComponent(userRepo) + '/pipelines'; | ||
| console.log(apiUrl); | ||
| var badgeData = getBadgeData({'s':'build', 'c': 'coverage'}[dataKind], data); | ||
| var callback = function(branch, page){ |
There was a problem hiding this comment.
This code is a bit difficult to follow, and may need to be reworked, it seems.
|
I tend to make this badge to have better behavior than the official one, since otherwise there's no point to use shields instead of their svg badge. I suspect they may implement rate limiting in the future, and copied the flexible authentication module from GitHub. Their API sucks hard (and the documentation too), and at least two requests are required; I hate the idea of defaulting to master, since that there's many projects that use a different workflow. I have no time to work on this for the moment, so I will update when I have free time. |
|
If defaulting to master seems like a bad idea, maybe the best bet is to require the user to specify the branch. I'll hold off until I hear from you. |
|
Will this support custom gitlab installations? Would be really cool to be able to specify the root URL. Also, see https://gitlab.com/gitlab-org/gitlab-ce/issues/30120#note_26962652 for discussion with the Gitlab team about this. |
That would be good to add! Some other badges already support something similar, for example the Jenkins badge supports it (eg. https://img.shields.io/jenkins/s/https/build.dan.cx/job/yarn-e2e.svg), as does the TeamCity badge (eg. https://img.shields.io/teamcity/http/teamcity.jetbrains.com/s/bt345.svg) |
|
I'm looking for someone to takeover the ownership of this change: simply comment here, and I will transfer my fork to you. I have no time to work on this. |
|
Thanks @ishitatsuyuki. Since this needs significant work, another option is to link to it from #541 and close it out. The code will be here, attached to the PR, whenever someone wants to pick it up. |
|
@blahah Would you like to pick this up? There's an issue which needs to be solved, I think with gitlab, so that this can make a finite, small number of API calls. |
|
@paulmelnikow happy to - it will be a few days before I can commit time to it, but after that I'll push it over the finish line. Thanks to everyone who's worked on this so far. |
|
@blahah I have requested for a transfer to you. Hope you can have luck doing this. I will still be subscribed for a while, so comment in case of problems. |
|
Wanted to mention we have a new service testing framework, so it would be good if the new version of this PR included tests: https://github.com/badges/shields/tree/master/service-tests |
|
@blahah would you have some time to address these comments and add tests? Please feel free to grab the branch, add more commits addressing the comments, and open a new PR referencing this one. If anyone else would like to adopt it, please feel free to do the same. Will close in one month if no one has. |
paulmelnikow
added
the
on-hold
|
Closing for inactivity. If you're reading this and want this implemented, don't be discouraged. You can grab the commits from the pull request branch using |
Address #541
A dirty but error-prone implementation.