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

Support untyped warnings from registries with successful publish #6303

Merged
merged 1 commit into from
Nov 12, 2018
Merged

Support untyped warnings from registries with successful publish #6303

merged 1 commit into from
Nov 12, 2018

Conversation

carols10cents
Copy link
Member

This adds a field "other" to the warnings deserialized from a successful publish response from a registry.

This is part of our plan to eventually require an email address to publish on crates.io to comply with DMCA. The TL;DR of that is we plan to warn for a release cycle when you publish without a verified email address once this change makes it to stable.

I'm opting to add an "other" field rather than another field like the invalid badges/categories fields for a few reasons:

  • The warning we're planning on adding about emails will only exist for 6 weeks; those other warnings have happened in the past and will continue to happen.
  • There may be other transient warnings on publish that we'd like to send from crates.io in the future; it'd be nice to have a way of doing that without having to update cargo as well.
  • Other registries may have different warnings than we could ever anticipate in cargo; if usage of alternate registries grows, it'd be nice to give them a mechanism to warn as well.

I've tested:

  • Cargo compiled with this change against a crates.io instance that doesn't return other warnings
  • Cargo compiled with this change against a crates.io instance that DOES return other warnings
  • Current Cargo against a crates.io instance that does return other warnings

and they all behaved as I expected.

I haven't added any tests because there aren't any tests that inject registry responses, and while I think cargo should have some of those eventually, I'm not going to add that infrastructure without discussing it with lots of folks first :)

I know there's a soft feature freeze right now, buuuuut it's wafer thin!! It doesn't add any surface area to the CLI or manifest format! ❤️

@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@carols10cents carols10cents changed the title Support untyped warnings from crates.io with successful publish Support untyped warnings from registries with successful publish Nov 11, 2018
@alexcrichton
Copy link
Member

@bors: r+

This looks good to me, thanks!

Is this something we should look to backport to get this onto stable more quickly?

@bors
Copy link
Collaborator

bors commented Nov 12, 2018

📌 Commit e66c413 has been approved by alexcrichton

@bors
Copy link
Collaborator

bors commented Nov 12, 2018

⌛ Testing commit e66c413 with merge 4e6bbd7...

bors added a commit that referenced this pull request Nov 12, 2018
…xcrichton

Support untyped warnings from registries with successful publish

This adds a field "other" to the warnings deserialized from a successful publish response from a registry.

This is [part of our plan to eventually require an email address to publish on crates.io to comply with DMCA](rust-lang/crates-io-cargo-teams#8). The TL;DR of that is we plan to warn for a release cycle when you publish without a verified email address once this change makes it to stable.

I'm opting to add an "other" field rather than another field like the invalid badges/categories fields for a few reasons:

- The warning we're planning on adding about emails will only exist for 6 weeks; those other warnings have happened in the past and will continue to happen.
- There may be other transient warnings on publish that we'd like to send from crates.io in the future; it'd be nice to have a way of doing that without having to update cargo as well.
- Other registries may have different warnings than we could ever anticipate in cargo; if usage of alternate registries grows, it'd be nice to give them a mechanism to warn as well.

I've tested:

- Cargo compiled with this change against a crates.io instance that doesn't return `other` warnings
- Cargo compiled with this change against a crates.io instance that DOES return `other` warnings
- Current Cargo against a crates.io instance that does return `other` warnings

and they all behaved as I expected.

I haven't added any tests because there aren't any tests that inject registry responses, and while I think cargo should have some of those eventually, I'm not going to add that infrastructure without discussing it with lots of folks first :)

I know there's a soft feature freeze right now, buuuuut [it's wafer thin](https://proxy.duckduckgo.com/iu/?u=https%3A%2F%2Fs4.thcdn.com%2Fproductimg%2F0%2F600%2F600%2F27%2F10284327-1288263770-74000.jpg&f=1)!! It doesn't add any surface area to the CLI or manifest format! ❤️
@bors
Copy link
Collaborator

bors commented Nov 12, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 4e6bbd7 to master...

@bors bors merged commit e66c413 into rust-lang:master Nov 12, 2018
@carols10cents
Copy link
Member Author

carols10cents commented Nov 12, 2018 via email

@alexcrichton
Copy link
Member

Ah ok, in that case I think we'll let this one ride the trains

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants