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

Fix [crates] license badge #1062

Merged
merged 2 commits into from
Sep 24, 2017
Merged

Fix [crates] license badge #1062

merged 2 commits into from
Sep 24, 2017

Conversation

iKevinY
Copy link
Contributor

@iKevinY iKevinY commented Aug 27, 2017

License data was moved to crate versions in rust-lang/crates.io#803. Closes #1059.

@paulmelnikow
Copy link
Member

Hi! Thanks for fixing this. Would love to get this merged. Could you add a test?

Copy link
Member

@paulmelnikow paulmelnikow left a comment

Choose a reason for hiding this comment

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

See comment.

@iKevinY
Copy link
Contributor Author

iKevinY commented Sep 22, 2017

@paulmelnikow Added a couple tests; let me know if I should make any changes!

@paulmelnikow paulmelnikow changed the title Fix crates.io license badge Fix [crates] license badge Sep 24, 2017
Copy link
Member

@paulmelnikow paulmelnikow left a comment

Choose a reason for hiding this comment

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

Unrelated unit tests are failing, though I think the problem was fixed a while ago in master. Could you try merging master into your branch to see if that fixes it?

.get('/l/libc.json')
.expectJSONTypes(Joi.object().keys({
name: Joi.equal('license'),
value: Joi.not('undefined')
Copy link
Member

Choose a reason for hiding this comment

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

Could you just use 'MIT/Apache-2.0' as before? Or, if for some reason this package's license is likely to change, choose a different one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! I don't anticipate the license to change, since the dual MIT/Apache-2.0 license is pretty standard across Rust projects, and libc is currently the most download crate.

@paulmelnikow paulmelnikow added bug Bugs in badges and the frontend service-badge Accepted and actionable changes, features, and bugs labels Sep 24, 2017
@iKevinY
Copy link
Contributor Author

iKevinY commented Sep 24, 2017

@paulmelnikow Hmm, I rebased on top of master, but it looks like the build is still failing. 😕

@paulmelnikow
Copy link
Member

That 403 Forbidden error was due to #979, so I restarted the build.

After that, we've got real failures. Those github failures are still there, which need investigation, plus there's a codetally failure, which I reported at #1069. Clearly those are unrelated to this change.

Copy link
Member

@paulmelnikow paulmelnikow left a comment

Choose a reason for hiding this comment

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

Thanks!

@paulmelnikow paulmelnikow merged commit c1a8166 into badges:master Sep 24, 2017
@paulmelnikow
Copy link
Member

Those github tests are passing on my machine. So the most likely explanation is that they are also related to #979, in a different manifestation, where the rate limit runs out, affecting some of the requests in the middle of the suite, and not others. It's a little odd that some github tests pass afterward, though some of them are mocked, and others might not be subject to rate limits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs in badges and the frontend service-badge Accepted and actionable changes, features, and bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants