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

Cargo.toml: exclude more CI files #1311

Closed
wants to merge 1 commit into from
Closed

Conversation

elmarco
Copy link
Contributor

@elmarco elmarco commented Apr 30, 2020

No description provided.

Signed-off-by: Marc-André Lureau <[email protected]>
@Thomasdezeeuw
Copy link
Collaborator

Maybe we should use include instead, something like include = ["/Cargo.toml", "/src/**/*.rs", "/README.md", "/LICENSE"]?

@elmarco
Copy link
Contributor Author

elmarco commented Apr 30, 2020

I leave that decision up to the maintainers

@Thomasdezeeuw
Copy link
Collaborator

I leave that decision up to the maintainers

Well that would be me :)

I've tested it locally and we need the examples as well:

include       = ["/Cargo.toml", "/src/**/*.rs", "/examples", "/README.md", "/LICENSE"]

@carllerche @kleimkuhler any opinions about excluding tests and additional files from the package uploaded to crates.io?

@asomers
Copy link
Collaborator

asomers commented Apr 30, 2020

I leave that decision up to the maintainers

Well that would be me :)

I've tested it locally and we need the examples as well:

include       = ["/Cargo.toml", "/src/**/*.rs", "/examples", "/README.md", "/LICENSE"]

@carllerche @kleimkuhler any opinions about excluding tests and additional files from the package uploaded to crates.io?

What are the examples needed for?

@Thomasdezeeuw
Copy link
Collaborator

@asomers Cargo.toml has two [[example]] sections and cargo commands will fail if the files are missing, i.e. cargo build fails with:

error: failed to parse manifest at `mio/Cargo.toml`

Caused by:
  can't find `tcp_server` example, specify example.path

@kleimkuhler
Copy link
Contributor

I don't have a major preference on exclude vs include. If we go with exclude and start excluding tests, is that common or necessary? I would assume the #[test] attribute already excludes in publishing, so doing it on tests/ may not be necessary?

@Thomasdezeeuw
Copy link
Collaborator

The exclude/include attribute determines what will be packaged and shipped to crates.io and end-users. Its not really critical but in most cases you really only need the source code and license. Removing the tests and other files makes the package smaller.

@elmarco
Copy link
Contributor Author

elmarco commented Apr 30, 2020

It also makes packaging for distros a bit easier. Fedora packaging is what prompted me to make this patch.

@kleimkuhler
Copy link
Contributor

Sounds good. Again I don't have a major preference; I lean towards the include that @Thomasdezeeuw suggested above, but would also be good with the current change!

@pfmooney pfmooney mentioned this pull request May 1, 2020
@Thomasdezeeuw
Copy link
Collaborator

Thomasdezeeuw commented May 1, 2020

@elmarco can you change this to match 6df2fb1?

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.

4 participants