badges / shields

Concise, consistent, and legible badges in SVG and raster format
https://shields.io
Creative Commons Zero v1.0 Universal
23.85k stars 5.51k forks source link

Numeric CPAN versions show as invalid #1599

Closed dmtucker closed 4 years ago

dmtucker commented 6 years ago

Some projects on CPAN have numeric versions, and they show up as invalid on the CPAN badge.

For example, https://fastapi.metacpan.org/v1/release/App-Netsync: license The license seems fine, version but the version won't show (since fixed).

Here are some more examples of projects that don't use strings: version https://fastapi.metacpan.org/v1/release/Async-Interrupt version https://fastapi.metacpan.org/v1/release/Crypt-SSSS version https://fastapi.metacpan.org/v1/release/Games-Literati version https://fastapi.metacpan.org/v1/release/MojoX-Redis

Here are some examples of projects that use strings: version https://fastapi.metacpan.org/v1/release/Apache-SPARQL version https://fastapi.metacpan.org/v1/release/App-gh version https://fastapi.metacpan.org/v1/release/App-Netdisco version https://fastapi.metacpan.org/v1/release/Test-CheckManifest

I suspect this is because versionColor raises a TypeError when trying to call toLowerCase: https://github.com/badges/shields/blob/435903ae4a170ffce188141f9c4ddafbc6565a43/lib/color-formatters.js#L14

var version = 3.01
version.toLowerCase()  // TypeError: version.toLowerCase is not a function

Since the MetaCPAN API offers version_numified alongside the version, I would guess this is actually a bug on their end, but the badges could probably work around it pretty easily.

PyvesB commented 6 years ago

Hello @dmtucker !

Thank you for spotting and reporting this. Even though it indeed seems like a bug on the CPAN end, I would be inclined to also cover this in Shields, as this may well happen with other APIs in the future. Does anyone else in the team have an opinion on this matter?

Cheers,

Pyves

paulmelnikow commented 6 years ago

Yea, agreed, let's make the version color formatter accept numbers. I wonder if it this is a regression from the changes we made in 7039e68…

PyvesB commented 6 years ago

@dmtucker would you like to work on a fix in Shields and submit a pull request? I'm happy to tackle it otherwise. 😉

dmtucker commented 6 years ago

@PyvesB Sounds interesting! I'm a bit out of my depth here, though, and I'm not sure when I could get to it. So, please, feel free!

PyvesB commented 6 years ago

I have just merged #1615, it will hopefully be released to production in the coming days/weeks. Here's the result in the staging environment for one of the previously failing badges:

@dmtucker do you think there's anything to add here?

dmtucker commented 6 years ago

Great! Nope, I don't think so. I suspected there was a similar condition in the version text logic, so I'm glad #1620 has gone in too :+1: Thanks for doing those!

PyvesB commented 6 years ago

Awesome, closing the issue then, thanks again for reporting it!

pryrt commented 4 years ago

Sorry to re-vivify this old, closed issue... but since I found it because I was trying to add badges to my Games-Literati (which happened to be one of the broken ones mentioned above!) and found this issue when trying to figure out why the badge doesn't work: is the "invalid response data" the fix from above, or did this bug come back?

Add a screenshot, since the live links may change over time: image

Since I am doing a release soon anyway, I might just go ahead and make my version scheme match my projects that are working, but thought I'd ask

(let me know if I need to create a new issue instead)

chris48s commented 4 years ago

The error being thrown here is ValidationError: "version" must be a string I think we need to relax the schema in https://github.com/badges/shields/blob/fafb22efee005122be095c4f74212f01df92c139/services/cpan/cpan.js#L7

PyvesB commented 4 years ago

Thanks for the report, I'll look into this again.