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

Separate licenses with a / instead of OR #5074

Merged
merged 1 commit into from
Feb 25, 2018
Merged

Separate licenses with a / instead of OR #5074

merged 1 commit into from
Feb 25, 2018

Conversation

CAD97
Copy link
Contributor

@CAD97 CAD97 commented Feb 25, 2018

It's what we tell others to do.

https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata

# This is a string description of the license for this package. Currently
# crates.io will validate the license provided against a whitelist of known
# license identifiers from http://spdx.org/licenses/. Multiple licenses can be
# separated with a `/`.
license = "..."

Noted here.

This should either use / or a note should be added that OR is allowed as a combination operator.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @matklad (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented Feb 25, 2018

📌 Commit 1ddba76 has been approved by alexcrichton

@bors
Copy link
Collaborator

bors commented Feb 25, 2018

⌛ Testing commit 1ddba76 with merge ed8dfce...

bors added a commit that referenced this pull request Feb 25, 2018
Separate licenses with a `/` instead of ` OR `

It's what we tell others to do.

https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata

> ```toml
> # This is a string description of the license for this package. Currently
> # crates.io will validate the license provided against a whitelist of known
> # license identifiers from http://spdx.org/licenses/. Multiple licenses can be
> # separated with a `/`.
> license = "..."
> ```

Noted [here](TeXitoi/structopt#71 (comment)).

This should either use `/` or a note should be added that ` OR ` is allowed as a combination operator.
@bors
Copy link
Collaborator

bors commented Feb 25, 2018

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

@bors bors merged commit 1ddba76 into rust-lang:master Feb 25, 2018
@ehuss
Copy link
Contributor

ehuss commented Feb 25, 2018

I think this is the opposite of what others have been transitioning towards. #4898 explicitly marked / as being deprecated.

@matklad
Copy link
Member

matklad commented Feb 25, 2018

Yeah, it seems @ehuss is right! #4920 switched / to OR some time back =P

I wonder if we should implement a warning on cargo publish

@ehuss
Copy link
Contributor

ehuss commented Feb 25, 2018

@matklad I can't find an explicit issue for issuing deprecation warnings, but I think the intent is to do that, but is maybe stalled on updating the license API to get those warnings? @wking may know more about that.

@wking
Copy link
Contributor

wking commented Feb 25, 2018

I think we want to revert this change and return to the SPDX-specified OR instead of the deprecated, Cargo-specific /.

I can't find an explicit issue for issuing deprecation warnings, but I think the intent is to do that, but is maybe stalled on updating the license API to get those warnings?

There's some discussion of warning in #2039. And yeah, license-exprs would need to grow an API that allowed reporting warnings (slightly more on that around here). license-exprs seems to still be stuck in a maintainer transition though (ehuss/license-exprs#12), so it may be a while before it grows such an API.

@matklad
Copy link
Member

matklad commented Feb 26, 2018

Aaaaand switching this back to OR in #5080 :)

@CAD97
Copy link
Contributor Author

CAD97 commented Feb 26, 2018

My bad, mayhaps update the documentation in prod :)

@CAD97 CAD97 deleted the patch-1 branch March 29, 2018 20:10
@ehuss ehuss added this to the 1.26.0 milestone Feb 6, 2022
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.

7 participants