Added [codetally] to shields project. #1069
Conversation
|
Prior build failure appeared to be due to call to github(?). Retrying. #979. |
|
#1054 would be awesome in addition to this merge. Thanks! |
paulmelnikow
changed the title
| .get('/triggerman722/colorstrap.json') | ||
| .expectJSONTypes(Joi.object().keys({ | ||
| name: Joi.equal('codetally'), | ||
| value: Joi.string().regex(/\b\d+(?:.\d+)?/) |
There was a problem hiding this comment.
Could you add a comment indicating the intent of this regex?
server.js
Outdated
| } | ||
| try { | ||
| var data = JSON.parse(buffer); | ||
| badgeData.text[1] = " " + data.currency_sign + " " + data.amount + " " + data.multiplier; |
There was a problem hiding this comment.
Could you omit the space between the currency sign and the amount? $2.50 instead of $ 2.50.
logo/codetally-white.svg
Outdated
| <rect x="6" y="3" width="1" height="7"/> | ||
| <line x1="0.25" y1="9.75" x2="6.75" y2="3.25" stroke-width="1" stroke="#fff" /> | ||
| </g> | ||
| </svg> |
There was a problem hiding this comment.
There isn't consensus about how to handle showing vendor logos, particularly by default. The design goal of shields was to display clean, consistent badges. The social badges have logos, though there are only a couple other badges with logos.
Would you mind removing it until a consensus is reached? It would expedite getting this merged.
There was a problem hiding this comment.
Thank you Paul. I will update to remove the logo, space and provide testing comments this and re-submit.
|
Hi Paul, I've made the 3 requested changes:
Note: the travis tests fail, however that is due to a 403 from github which occurs periodically due to rate limiting (i.e. not specific to this change). |
|
Thanks! I just re-ran the Travis build and one of the new Codetally tests failed. Ran it one more time, and it passed. Hopefully a fluke! |
|
Just caught another failure on that test in a different CI build. I timed the request by running Could there be some caching on the server that makes it work on subsequent requests, but not the first one? It seems to me that, to keep this badge, we need their API to respond faster than two seconds. Even 100–200 ms is a bit long. Could you raise the issue upstream? |
|
Hi Paul, Codetally is on heroku and yesterday I upgraded my plan there to avoid this problem. On Heroku's cheap plans (which is what Codetally was on when you saw this problem) sites with low volume get put to sleep, then fire up on first request. That is why you had 2s then 100-200ms. From now on, you should have neither problem (i.e. no more initial lag and individual hits should be faster as well.). If we see a continued trend of poor performance, please log an issue in the codetally_core project - https://github.com/Codetally/codetally_core/issues/new. Thanks |
|
Aha! It was a Hobby dyno. Thanks for addressing that. |
Looking to add codetally to the shields project.