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

use max_stable_version in rust [crates] badge #8687

Merged
merged 2 commits into from
Feb 25, 2023

Conversation

chris48s
Copy link
Member

@chris48s chris48s commented Dec 4, 2022

Refs #8666
Refs #8667

This PR switches us to using max_stable_version if present, or max_version otherwise, as suggested in #8666

While I was looking at this, I realised returning json.version.num makes no sense here and we should never take this branch. It is a hangover from a really really old implementation

shields/server.js

Lines 459 to 528 in 302c860

// Rust download and version integration
camp.route(/^\/crates\/(d|v|dv|l)\/([A-Za-z0-9_-]+)(?:\/([0-9.]+))?\.(svg|png|gif|jpg|json)$/,
cache(function (data, match, sendBadge, request) {
var mode = match[1]; // d - downloads (total or for version), v - (latest) version, dv - downloads (for latest version)
var crate = match[2]; // crate name, e.g. rustc-serialize
var version = match[3]; // crate version in semver format, optional, e.g. 0.1.2
var format = match[4];
var modes = {
'd': {
name: 'downloads',
version: true,
process: function (data, badgeData) {
var downloads = data.crate? data.crate.downloads: data.version.downloads;
version = data.version && data.version.num;
badgeData.text[1] = metric(downloads) + (version? ' version ' + version: '');
badgeData.colorscheme = downloadCountColor(downloads);
},
},
'dv': {
name: 'downloads',
version: true,
process: function (data, badgeData) {
var downloads = data.version? data.version.downloads: data.versions[0].downloads;
version = data.version && data.version.num;
badgeData.text[1] = metric(downloads) + (version? ' version ' + version: ' latest version');
badgeData.colorscheme = downloadCountColor(downloads);
},
},
'v': {
name: 'crates.io',
version: true,
process: function (data, badgeData) {
version = data.version? data.version.num: data.crate.max_version;
badgeData.text[1] = versionText(version);
badgeData.colorscheme = versionColor(version);
},
},
'l': {
name: 'license',
version: false,
process: function (data, badgeData) {
badgeData.text[1] = data.versions[0].license;
badgeData.colorscheme = 'blue';
},
},
};
var behavior = modes[mode];
var apiUrl = 'https://crates.io/api/v1/crates/' + crate;
if (version != null && behavior.version) {
apiUrl += '/' + version;
}
var badgeData = getBadgeData(behavior.name, data);
request(apiUrl, { headers: { 'Accept': 'application/json' } }, function (err, res, buffer) {
if (err != null) {
badgeData.text[1] = 'inaccessible';
sendBadge(format, badgeData);
return;
}
try {
var data = JSON.parse(buffer);
behavior.process(data, badgeData);
sendBadge(format, badgeData);
} catch (e) {
badgeData.text[1] = 'invalid';
sendBadge(format, badgeData);
}
});
}));
and we can get rid of it. We only ever call the /api/v1/crates/:crate endpoint when we render this badge - never the /api/v1/crates/:crate/:version endpoint, so we don't need to handle that response.

@chris48s chris48s added the service-badge Accepted and actionable changes, features, and bugs label Dec 4, 2022
@shields-ci
Copy link

Warnings
⚠️ This PR modified service code for crates but not its test code.
That's okay so long as it's refactoring existing code.
Messages
📖 ✨ Thanks for your contribution to Shields, @chris48s!

Generated by 🚫 dangerJS against 86b8a54

@zeeshanlakhani
Copy link
Contributor

zeeshanlakhani commented Dec 4, 2022

@chris48s +1. I had similar Qs on the json.version.num, but wasn't sure the history.

Copy link
Collaborator

@PaulaBarszcz PaulaBarszcz left a comment

Choose a reason for hiding this comment

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

👍

@zeeshanlakhani
Copy link
Contributor

@chris48s just wanted to check in on this?

@calebcartwright
Copy link
Member

@chris48s just wanted to check in on this?

This is pending on me, and through a mixture of forgetfulness and not having adequate bandwidth, I haven't gotten round to it yet.

Will try to soon though 🤞

Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

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

Confirmed this is the route we want to go, apologies for this being held up waiting for action from my end!

@repo-ranger repo-ranger bot merged commit 088fb17 into badges:master Feb 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service-badge Accepted and actionable changes, features, and bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants