Skip to content
This repository has been archived by the owner on Dec 25, 2022. It is now read-only.

Updated sec-1 to match ping example code #39

Closed
wants to merge 4 commits into from
Closed

Updated sec-1 to match ping example code #39

wants to merge 4 commits into from

Conversation

raymondsiu
Copy link

I noticed the Getting Started section still uses Actix 0.8 and Futures but the ping example has moved on to use the newer async await standard without Futures.

I did run cargo test before committing and there's one outstanding failed test for sec-1:

test sec_1_getting_started_sect_ping_actor_line_115 ... FAILED
warning: struct is never constructed: `Ping`
 --> /tmp/rust-skeptic.Is0iZA6jfrsE/test.rs:5:1
  |
5 | struct Ping(usize);
  | ^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(dead_code)]` on by default

I'm not entirely sure, but I think this test failed due to skeptic.rs not being able to grok the new await syntax so it thinks the Ping struct wasn't used.

This is my first Rust-related PR so let me know if there's anything I missed! I would greatly appreciated the guidance!

@JohnTitor
Copy link
Member

This crate is edition 2015 now so we should make it edition 2018.

@raymondsiu
Copy link
Author

Where can I specify the 2018 edition?

@JohnTitor
Copy link
Member

You can specify the edition on Cargo.toml on the root.

@JohnTitor
Copy link
Member

We should update all examples at once to make CI green.

@@ -1,3 +1,5 @@
cargo-features = ["edition"]
Copy link
Member

Choose a reason for hiding this comment

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

Is it really needed?

@raymondsiu raymondsiu closed this Jan 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants