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

Port dom, fetch, guide-supported-types-examples, import_js examples to Rust 2018 edition #1103

Merged
merged 9 commits into from
Dec 11, 2018

Conversation

T5uku5hi
Copy link
Contributor

@T5uku5hi T5uku5hi commented Dec 11, 2018

This PR ports the following examples:

  • dom
  • fetch
  • guide-supported-types-examples
  • import_js

I run these command.

cargo fix --edition
cargo check --target wasm32-unknown-unknown
cargo fix --edition-idioms

Relates to #1099.

🐞 bug report

When I run $ cargo fix --edition-idioms in import_js, I got an error message.
full output:

import_js $ cargo fix --edition-idioms
    Checking import_js v0.1.0 (/Users/misakimakino/Works/clones/fork-wasm-bindgen/wasm-bindgen/examples/import_js)
warning: failed to automatically apply fixes suggested by rustc to crate `import_js`

after fixes were automatically applied the compiler reported errors within these files:

  * examples/import_js/src/lib.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/cargo/issues
quoting the full output of this command we'd be very appreciative!

warning: `extern crate` is not idiomatic in the new edition
 --> examples/import_js/src/lib.rs:1:1
  |
1 | extern crate wasm_bindgen;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: convert it to a `use`
  |
  = note: `-W unused-extern-crates` implied by `-W rust-2018-idioms`

warning: `extern crate` is not idiomatic in the new edition
 --> examples/import_js/src/lib.rs:1:1
  |
1 | extern crate wasm_bindgen;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: convert it to a `use`
  |
  = note: `-W unused-extern-crates` implied by `-W rust-2018-idioms`

    Finished dev [unoptimized + debuginfo] target(s) in 1.05s
import_js $

@fitzgen
Copy link
Member

fitzgen commented Dec 11, 2018

bug report

When I run $ cargo fix --edition-idioms in import_js, I got an error message.
full output:

FWIW, it is probably best to file this upstream in cargo's issue tracker!

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

LGTM with the little nitpicks below addressed :)

extern crate wasm_bindgen;
extern crate web_sys;

use web_sys;
Copy link
Member

Choose a reason for hiding this comment

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

This line shouldn't be necessary in 2018 edition either :)

@@ -1,6 +1,6 @@
#![allow(unused_variables, dead_code)]

extern crate wasm_bindgen;
use wasm_bindgen;
Copy link
Member

Choose a reason for hiding this comment

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

Ditto here and elsewhere. In general, we don't need use some_crate; in order to do some_crate::Something.

@T5uku5hi
Copy link
Contributor Author

@fitzgen
Thank you for your comment.
I removed unnecessary lines.

extern crate js_sys;
extern crate wasm_bindgen;
extern crate wasm_bindgen_futures;
extern crate web_sys;
#[macro_use]
extern crate serde_derive;
Copy link
Contributor

Choose a reason for hiding this comment

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

If you're extra ambitious, we can actually remove serde_derive both here and in Cargo.toml by adding this to Cargo.toml:

[dependencies]
serde = { version = "1.0.80", features = ["derive"] }

and then adding here:

use serde::{Deserialize, Serialize};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexcrichton
Thank you for your comment.
I'll try it !!

@T5uku5hi
Copy link
Contributor Author

I added serde = { version = "1.0.80", features = ["derive"] } in Cargo.toml.

@alexcrichton alexcrichton merged commit 82b322a into rustwasm:master Dec 11, 2018
@alexcrichton
Copy link
Contributor

👍

Thanks!

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