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

Bring tide to root, and allow cargo run --example #247

Merged
merged 5 commits into from
May 21, 2019
Merged

Conversation

yoshuawuyts
Copy link
Member

Description

Moves the tide submodule to the root of the crate, allows running examples using:

$ cargo run --example <example>

Also makes the examples to be run standalone without a need for lib.rs. Ref #223.

Motivation and Context

This patch is an alternative to #227. As explained in #223 (comment), I think it's preferable if we keep Cargo's default behavior of being able to run examples using cargo run --example.

I spent some time today trying to figure out how to run our examples, only to find out that they weren't runnable, which was unexpected, and admittedly rather frustrating.

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Signed-off-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
Signed-off-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
Signed-off-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
@prasannavl
Copy link
Contributor

Awesome! I do think this is much nicer, and also keeps things consistent with runtime. My initial motivation to not go this route, was that we may have to tweak tooling scaffolds and other experience if porting over from futures, tokio, etc.

But yeah, we can take the new path and figure things out as we go :)

@fairingrey
Copy link
Contributor

This approach is neat, reminds me of the way actix-web is laid out (with the main crate at root, so we no longer rely on a workspace manifest alone). I like it 👍

PS: Thanks for fixing my example!

`tide` now sits at root of crate
@prasannavl
Copy link
Contributor

If the travis bit can be fixed, I think we can merge this and handle the example clippy warnings post merge, for this particular instance.

@fairingrey
Copy link
Contributor

@prasannavl Yup, after these Travis runs finish, clippy should only be yelling at the examples (and we can just get those fixed up in a later PR)

@fairingrey
Copy link
Contributor

Clippy freaks out on the examples, but as mentioned before we can fix it later -- merging!

@fairingrey fairingrey merged commit 5ec263e into master May 21, 2019
@delete-merged-branch delete-merged-branch bot deleted the run-examples branch May 21, 2019 05:14
fairingrey added a commit that referenced this pull request May 21, 2019
`examples` is no longer a package in the workspace following #247
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.

3 participants