Merge branch 'master' into avery/bob-the-builder-becomes-more-efficie…

…nt-by-laying-off-half-of-his-workforce

CONTRIBUTING.md

@@ -18,11 +18,11 @@ Within 3 days, any incoming issue should be triaged. Triage involves:
 
 ### Labelling
 
-- label all issues coming from non-team members with User Report
-- labelling the category of the issue: Feature, External Bug, Bug, Maintenance, Docs
-- optionally labelling a secondary category: Webpack, Routes, Workers Runtime, Refactor
-- labelling the status of the issue: Need More Info, Needs Repro, Needs Design, PR Welcome
-- optionally labelling other calls to action: Help Wanted, Question
+- label all issues coming from non-team members with `user report`
+- labelling the category of the issue: `feature`, `external bug`, `bug`, `maintenance`, `docs`, `refactor`, `release`
+- labelling the status of the issue: `needs design`, `needs docs`, `needs more info`, `needs repro`, `needs template`, `PR attached`, `PR welcome`, `waiting on response`
+- optionally labelling a subject: `cargo install`, `kv`, `routes`, `site`, `webpack`, `workers runtime`
+- optionally labelling other calls to action: `help wanted`, `question`, `good first issue`
 
 ### Assignment
 
@@ -36,7 +36,7 @@ our plans for the milestones and releases.
 
 ### Labelling
 
-- labelling the priority of the issue: Critical, Nice to Have, Low Priority
+- labelling the priority of the issue: `critical`, `nice to have`, `low priority`
 - labelling the status of the issue: Needs Design, PR Welcome
 
 ### Assignment and Milestones
@@ -51,12 +51,12 @@ should be triaged immediately upon open by the PR author.
 
 ### Labelling
 
-- All work-in-progress PRs should be labelled Work In Progress and the title should be
+- All work-in-progress PRs should be labelled `work in progress` and the title should be
     annotated [WIP] for easy scanning. No WIP PRs will be reviewed until the annotations
     are removed.
-- All PRs that need to be reviewed should be labelled Needs Review until they have 
+- All PRs that need to be reviewed should be labelled `needs review` until they have 
     received all required reviews.
-- All PRs should be labelled with a changelog label: BREAKING, Feature, Bug, Maintenance, Docs
+- All PRs should be labelled with a changelog label: `BREAKING`, `feature`, `fix`, `maintenance`, `docs`
 
 ### Merging
 

npm/install-wrangler.js

@@ -2,7 +2,6 @@ const axios = require("axios");
 const os = require("os");
 const { join, resolve } = require("path");
 const { mkdirSync, existsSync } = require("fs");
-// while recent versions of Node can do that natively, wait until we can use it.
 const rimraf = require("rimraf");
 const tar = require("tar");
 const { get } = axios;

src/commands/build/watch/mod.rs

@@ -20,8 +20,8 @@ const RUST_PATH: &str = "./";
 // Paths to ignore live watching in Rust Workers
 const RUST_IGNORE: &'static [&str] = &["pkg", "target", "worker/generated"];
 
-/// watch a project for changes and re-build it when necessary,
-/// outputting a build event to tx.
+// watch a project for changes and re-build it when necessary,
+// outputting a build event to tx.
 pub fn watch_and_build(
     target: &Target,
     tx: Option<mpsc::Sender<()>>,

src/commands/build/watch/watcher.rs

@@ -8,7 +8,7 @@ use failure::{format_err, Error};
 use crate::terminal::message;
 use log::info;
 
-/// Add cooldown for all types of events to watching logic
+// Add cooldown for all types of events to watching logic
 pub fn wait_for_changes(
     rx: &Receiver<DebouncedEvent>,
     cooldown: Duration,

src/commands/build/wranglerjs/bundle.rs

@@ -55,10 +55,6 @@ impl Bundle {
         self.wasm_path().exists()
     }
 
-    pub fn has_webpack_config(&self, webpack_config_path: &PathBuf) -> bool {
-        webpack_config_path.exists()
-    }
-
     pub fn get_wasm_binding(&self) -> String {
         "wasm".to_string()
     }

src/commands/build/wranglerjs/mod.rs

@@ -30,9 +30,9 @@ use std::thread;
 use std::time::Duration;
 
 // Run the underlying {wranglerjs} executable.
-//
-// In Rust we create a virtual file, pass the pass to {wranglerjs}, run the
-// executable and wait for completion. The file will receive the a serialized
+
+// In Rust we create a virtual file, pass it to {wranglerjs}, run the
+// executable and wait for completion. The file will receive a serialized
 // {WranglerjsOutput} struct.
 // Note that the ability to pass a fd is platform-specific
 pub fn run_build(target: &Target) -> Result<(), failure::Error> {
@@ -43,8 +43,8 @@ pub fn run_build(target: &Target) -> Result<(), failure::Error> {
     let status = command.status()?;
 
     if status.success() {
-        let output = fs::read_to_string(temp_file).expect("could not retrieve output");
-
+        let output = fs::read_to_string(&temp_file).expect("could not retrieve output");
+        fs::remove_file(temp_file)?;
         let wranglerjs_output: WranglerjsOutput =
             serde_json::from_str(&output).expect("could not parse wranglerjs output");
 
@@ -62,7 +62,7 @@ pub fn run_build_and_watch(target: &Target, tx: Option<Sender<()>>) -> Result<()
 
     info!("Running {:?} in watch mode", command);
 
-    //Turbofish the result of the closure so we can use ?
+    // Turbofish the result of the closure so we can use ?
     thread::spawn::<_, Result<(), failure::Error>>(move || {
         let _command_guard = util::GuardedCommand::spawn(command);
 
@@ -93,8 +93,8 @@ pub fn run_build_and_watch(target: &Target, tx: Option<Sender<()>>) -> Result<()
                     if is_first {
                         is_first = false;
                         message::info("Ignoring stale first change");
-                        //skip the first change event
-                        //so we don't do a refresh immediately
+                        // skip the first change event
+                        // so we don't do a refresh immediately
                         continue;
                     }
 
@@ -158,7 +158,8 @@ fn setup_build(target: &Target) -> Result<(Command, PathBuf, Bundle), failure::E
     command.arg(wranglerjs_path);
     let has_wasm_pack_plugin = true;
     if has_wasm_pack_plugin {
-        //put path to our wasm_pack as env variable so wasm-pack-plugin can utilize it
+        // export WASM_PACK_PATH for use by wasm-pack-plugin
+        // https://github.com/wasm-tool/wasm-pack-plugin/blob/caca20df84782223f002735a8a2e99b2291f957c/plugin.js#L13
         let tool_name = "wasm-pack";
         let author = "rustwasm";
         let wasm_pack_path = install::install(tool_name, author)?.binary(tool_name)?;
@@ -179,29 +180,28 @@ fn setup_build(target: &Target) -> Result<(Command, PathBuf, Bundle), failure::E
 
     command.arg(format!("--wasm-binding={}", bundle.get_wasm_binding()));
 
-    let webpack_config_path = if let Some(webpack_config) = &target.webpack_config {
-        // require webpack_config in wrangler.toml to use it in sites
-        Some(PathBuf::from(&webpack_config))
-    } else if target.site.is_none() {
-        let config_path = PathBuf::from("webpack.config.js".to_string());
-        // backwards compatibility, deprecated in 1.6.0
-        // if webpack.config.js exists and is not specified in wrangler.toml, use it and warn
-        if bundle.has_webpack_config(&config_path) {
-            message::warn("In Wrangler v1.6.0, you will need to include a webpack_config field in your wrangler.toml to build with a custom webpack configuration.");
-            Some(config_path)
-        } else {
-            // if webpack.config.js does not exist, don't warn, use our default
+    let custom_webpack_config_path = match &target.webpack_config {
+        Some(webpack_config) => match &target.site {
+            None => Some(PathBuf::from(&webpack_config)),
+            Some(_) => {
+                message::warn("Workers Sites does not support custom webpack configuration files");
+                None
+            }
+        },
+        None => {
+            if target.site.is_none() {
+                let config_path = PathBuf::from("webpack.config.js".to_string());
+                if config_path.exists() {
+                    message::warn("If you would like to use your own custom webpack configuration, you will need to add this to your wrangler.toml:\nwebpack_config = \"webpack.config.js\"");
+                }
+            }
             None
         }
-    } else {
-        // don't use `webpack.config.js` if this project is a site
-        None
     };
 
-    // if {webpack.config.js} is not present, we infer the entry based on the
-    // {package.json} file and pass it to {wranglerjs}.
-    // https://github.com/cloudflare/wrangler/issues/98
-    if let Some(webpack_config_path) = webpack_config_path {
+    // if webpack_config is not configured in the manifest
+    // we infer the entry based on {package.json} and pass it to {wranglerjs}
+    if let Some(webpack_config_path) = custom_webpack_config_path {
         build_with_custom_webpack(&mut command, &webpack_config_path);
     } else {
         build_with_default_webpack(&mut command, &build_dir)?;
@@ -234,7 +234,6 @@ fn build_with_default_webpack(
 
 pub fn scaffold_site_worker(target: &Target) -> Result<(), failure::Error> {
     let build_dir = target.build_dir()?;
-    // TODO: this is a placeholder template. Replace with The Real Thing on launch.
     let template = "https://github.com/cloudflare/worker-sites-init";
 
     if !Path::new(&build_dir).exists() {
@@ -272,7 +271,7 @@ fn run_npm_install(dir: &PathBuf) -> Result<(), failure::Error> {
         info!("skipping npm install because node_modules exists");
     }
 
-    // TODO(sven): figure out why the file doesn't exits in some cases?
+    // TODO: (sven) figure out why the file doesn't exist in some cases
     if flock_path.exists() {
         fs::remove_file(&flock_path)?;
     }

src/commands/build/wranglerjs/output.rs

@@ -6,8 +6,8 @@ use serde::Deserialize;
 use std::io::prelude::*;
 
 // This structure represents the communication between {wranglerjs} and
-// {wrangler}. It is send back after {wranglerjs} completion.
-// FIXME(sven): make this private
+// {wrangler}. It is sent back after {wranglerjs} completion.
+// TODO: (sven) make this private
 #[derive(Deserialize, Debug)]
 pub struct WranglerjsOutput {
     pub wasm: Option<String>,
@@ -28,7 +28,7 @@ impl WranglerjsOutput {
     fn project_size_bytes(&self) -> u64 {
         let mut e = ZlibEncoder::new(Vec::new(), Compression::default());
 
-        //approximation of how projects are gzipped
+        // approximation of how projects are gzipped
         e.write_all(&self.script.as_bytes())
             .expect("could not write script buffer");
 
@@ -42,7 +42,7 @@ impl WranglerjsOutput {
 
     fn project_size_message(compressed_size: u64) -> String {
         const MAX_PROJECT_SIZE: u64 = 1 << 20; // 1 MiB
-        const WARN_THRESHOLD: u64 = MAX_PROJECT_SIZE - 81_920; //Warn when less than 80 KiB left to grow, ~92% usage
+        const WARN_THRESHOLD: u64 = MAX_PROJECT_SIZE - 81_920; // Warn when less than 80 KiB left to grow, ~92% usage
         const MAX_BEFORE_WARN: u64 = WARN_THRESHOLD - 1;
 
         let bytes_left = MAX_PROJECT_SIZE.checked_sub(compressed_size);

src/commands/config/mod.rs

@@ -7,8 +7,7 @@ use std::path::PathBuf;
 
 use crate::settings::global_user::{get_global_config_dir, GlobalUser};
 
-// set the permissions on the dir, we want to avoid that other user reads to
-// file
+// set the permissions on the dir, we want to avoid other user reads of the file
 #[cfg(not(target_os = "windows"))]
 pub fn set_file_mode(file: &PathBuf) {
     File::open(&file)

src/commands/kv/bucket/upload.rs

@@ -172,7 +172,7 @@ mod tests {
             // Ensure the expected key and value was returned in the filtered pair list
             // Awkward field-by-field comparison below courtesy of not yet implementing
             // PartialEq for KeyValuePair in cloudflare-rs :)
-            // todo(gabbi): Implement PartialEq for KeyValuePair in cloudflare-rs.
+            // TODO: (gabbi) Implement PartialEq for KeyValuePair in cloudflare-rs.
             assert!(pair.key == actual[idx].key);
             assert!(pair.value == actual[idx].value);
             idx += 1;

src/commands/kv/key/get.rs

@@ -1,4 +1,4 @@
-// todo(gabbi): This file should use cloudflare-rs instead of our http::auth_client
+// TODO:(gabbi) This file should use cloudflare-rs instead of our http::auth_client
 // when https://github.com/cloudflare/cloudflare-rs/issues/26 is handled (this is
 // because the GET key operation doesn't return json on success--just the raw
 // value).

src/commands/kv/key/put.rs

@@ -1,4 +1,4 @@
-// todo(gabbi): This file should use cloudflare-rs instead of our http::auth_client
+// TODO: (gabbi) This file should use cloudflare-rs instead of our http::auth_client
 // when https://github.com/cloudflare/cloudflare-rs/issues/26 is handled (this is
 // because the SET key request body is not json--it is the raw value).
 

src/commands/mod.rs

@@ -22,7 +22,7 @@ pub use subdomain::get_subdomain;
 pub use subdomain::set_subdomain;
 pub use whoami::whoami;
 
-/// Run the given command and return its stdout.
+// Run the given command and return its stdout.
 pub fn run(mut command: Command, command_name: &str) -> Result<(), failure::Error> {
     log::info!("Running {:?}", command);
 

src/commands/preview/fiddle_messenger.rs

@@ -35,23 +35,23 @@ impl Handler for FiddleMessageServer {
 
         const SAFE_ADDRS: &[&str] = &["127.0.0.1", "localhost", "::1"];
 
-        //origin() returns Result<Option<&str>>
+        // origin() returns Result<Option<&str>>
         let origin = handshake
             .request
             .origin()?
             .unwrap_or("unknown")
             .trim_end_matches(|c: char| c == '/' || c == ':' || c.is_numeric());
 
-        //remote_addr returns Result<Option<String>>
+        // remote_addr returns Result<Option<String>>
         let incoming_addr = handshake.remote_addr()?;
         let incoming_addr = incoming_addr.as_ref().map_or("unknown", String::as_str);
 
-        //only allow connections from cloudflareworkers.com
+        // only allow connections from cloudflareworkers.com
         let origin_is_safe = SAFE_ORIGINS
             .iter()
             .any(|safe_origin| &origin == safe_origin);
 
-        //only allow incoming websocket connections from localhost/current machine.
+        // only allow incoming websocket connections from localhost/current machine.
         let addr_is_safe = SAFE_ADDRS
             .iter()
             .any(|safe_addr| &incoming_addr == safe_addr);

src/commands/preview/mod.rs

@@ -48,7 +48,8 @@ pub fn preview(
     let https_str = if https { "https://" } else { "http://" };
 
     if livereload {
-        let server = WebSocket::new(|out| FiddleMessageServer { out })?.bind("127.0.0.1:0")?; //explicitly use 127.0.0.1, since localhost can resolve to 2 addresses
+        // explicitly use 127.0.0.1, since localhost can resolve to 2 addresses
+        let server = WebSocket::new(|out| FiddleMessageServer { out })?.bind("127.0.0.1:0")?;
 
         let ws_port = server.local_addr()?.port();
 
@@ -61,7 +62,7 @@ pub fn preview(
         ))?;
         }
 
-        //don't do initial GET + POST with livereload as the expected behavior is unclear.
+        // don't do initial GET + POST with livereload as the expected behavior is unclear.
 
         let broadcaster = server.broadcaster();
         thread::spawn(move || server.run());

src/commands/publish/upload_form/mod.rs

@@ -56,8 +56,8 @@ pub fn build(
             build_form(&assets)
         }
         TargetType::Webpack => {
-            log::info!("Webpack project detected. Publishing...");
-            // FIXME(sven): shouldn't new
+            log::info!("webpack project detected. Publishing...");
+            // TODO: https://github.com/cloudflare/wrangler/issues/850
             let build_dir = target.build_dir()?;
             let bundle = wranglerjs::Bundle::new(&build_dir);
 

src/main.rs

@@ -409,8 +409,9 @@ fn run() -> Result<(), failure::Error> {
         let default = !matches.is_present("api-key");
 
         let user: GlobalUser = if default {
-            // Default: use API token.
-            message::info("Looking to use a Global API Key and email instead? Run \"wrangler config --api-key\". (Not Recommended)");
+            // API Tokens are the default
+            message::info("To find your API token, go to https://dash.cloudflare.com/profile/api-tokens and create it using the \"Edit Cloudflare Workers\" template");
+            message::info("If you are trying to use your Global API Key instead of an API Token (Not Recommended), run \"wrangler config --api-key\".");
             println!("Enter API token: ");
             let mut api_token: String = read!("{}\n");
             api_token.truncate(api_token.trim_end().len());
@@ -466,7 +467,7 @@ fn run() -> Result<(), failure::Error> {
         let name = matches.value_of("name");
         let site = matches.is_present("site");
         let target_type = if site {
-            // Workers Sites projects are always Webpack for now
+            // Workers Sites projects are always webpack for now
             Some(TargetType::Webpack)
         } else {
             match matches.value_of("type") {
@@ -563,7 +564,7 @@ fn run() -> Result<(), failure::Error> {
                     }
                     None => delete_matches
                         .value_of("namespace-id")
-                        .unwrap() // clap configs ensure that if "binding" isn't present,"namespace-id" must be.
+                        .unwrap() // clap configs ensure that if "binding" isn't present, "namespace-id" must be.
                         .to_string(),
                 };
                 commands::kv::namespace::delete(&target, &user, &namespace_id)?;

src/settings/target/manifest.rs

@@ -122,7 +122,7 @@ impl Manifest {
     }
 
     pub fn get_target(&self, environment_name: Option<&str>) -> Result<Target, failure::Error> {
-        // Site projects are always Webpack for now; don't let toml override this.
+        // Site projects are always webpack for now; don't let toml override this.
         let target_type = match self.site {
             Some(_) => TargetType::Webpack,
             None => self.target_type.clone(),

src/settings/target/site.rs

@@ -21,6 +21,9 @@ impl Site {
         site
     }
 
+    // if the user has configured `site.entry-point`, use that
+    // as the build directory. Otherwise use the default const
+    // SITE_ENTRY_POINT
     pub fn build_dir(&self, current_dir: PathBuf) -> Result<PathBuf, std::io::Error> {
         Ok(current_dir.join(
             self.entry_point

src/settings/target/target.rs

@@ -40,9 +40,6 @@ impl Target {
         let current_dir = env::current_dir()?;
         // if `site` is configured, we want to isolate worker code
         // and build artifacts away from static site application code.
-        // if the user has configured `site.entry-point`, use that
-        // as the build directory. Otherwise use the default const
-        // SITE_BUILD_DIR
         match &self.site {
             Some(site_config) => site_config.build_dir(current_dir),
             None => Ok(current_dir),

src/util.rs

@@ -1,9 +1,9 @@
 use std::process::{Child, Command};
 
-/// wrapper around spawning child processes such that they
-/// have the same behavior as spawned threads i.e. a spawned
-/// child process using GuardedChild has the same lifetime as
-/// the main thread.
+// wrapper around spawning child processes such that they
+// have the same behavior as spawned threads i.e. a spawned
+// child process using GuardedChild has the same lifetime as
+// the main thread.
 pub struct GuardedCommand(Child);
 
 impl GuardedCommand {