diff mbox

[v2,2/2] Fix clippy lint warnings and add exceptions for certain lints

Message ID 20161219032954.27792-2-andrew.donnellan@au1.ibm.com
State Superseded
Delegated to: Russell Currey
Headers show

Commit Message

Andrew Donnellan Dec. 19, 2016, 3:29 a.m. UTC
Fix a stack of warnings from clippy, and add a few #[allow]s for things
that are justifiable. No functional change.

Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

---

v1->v2:
* Wrap the #[allow]s with #[cfg_attr] so we can still build when clippy is 
disabled

---
 src/git.rs       | 10 +++++-----
 src/jenkins.rs   | 15 +++++++--------
 src/main.rs      | 36 +++++++++++++++++-------------------
 src/patchwork.rs |  2 ++
 src/settings.rs  |  2 +-
 5 files changed, 32 insertions(+), 33 deletions(-)
diff mbox

Patch

diff --git a/src/git.rs b/src/git.rs
index 679d387..9be867f 100644
--- a/src/git.rs
+++ b/src/git.rs
@@ -26,7 +26,7 @@  pub static GIT_REF_BASE: &'static str = "refs/heads";
 pub fn get_latest_commit(repo: &Repository) -> Commit {
     let head = repo.head().unwrap();
     let oid = head.target().unwrap();
-    return repo.find_commit(oid).unwrap();
+    repo.find_commit(oid).unwrap()
 }
 
 pub fn push_to_remote(remote: &mut Remote, branch: &str,
@@ -48,9 +48,9 @@  pub fn pull(repo: &Repository) -> Result<Output, &'static str> {
 
     if output.status.success() {
         debug!("Pull: {}", String::from_utf8(output.clone().stdout).unwrap());
-        return Ok(output);
+        Ok(output)
     } else {
-        return Err("Error: couldn't pull changes");
+        Err("Error: couldn't pull changes")
     }
 
 }
@@ -77,11 +77,11 @@  pub fn apply_patch(repo: &Repository, path: &Path)
     if output.status.success() {
         debug!("Patch applied with text {}",
                  String::from_utf8(output.clone().stdout).unwrap());
-        return Ok(output);
+        Ok(output)
     } else {
         Command::new("git").arg("am").arg("--abort")
             .current_dir(&workdir).output().unwrap();
-        return Err("Patch did not apply successfully");
+        Err("Patch did not apply successfully")
     }
 }
 
diff --git a/src/jenkins.rs b/src/jenkins.rs
index 6b92a85..1c63e75 100644
--- a/src/jenkins.rs
+++ b/src/jenkins.rs
@@ -73,6 +73,7 @@  impl CIBackend for JenkinsBackend {
     }
 }
 
+#[derive(Eq, PartialEq)]
 pub enum JenkinsBuildStatus {
     Running,
     Done,
@@ -131,20 +132,18 @@  impl JenkinsBackend {
     }
 
     pub fn get_build_status(&self, build_url: &str) -> JenkinsBuildStatus {
-        match self.get_api_json_object(build_url).get("building").unwrap().as_boolean().unwrap() {
-            true => JenkinsBuildStatus::Running,
-            false => JenkinsBuildStatus::Done,
+        if self.get_api_json_object(build_url)["building"].as_boolean().unwrap() {
+            JenkinsBuildStatus::Running
+        } else {
+            JenkinsBuildStatus::Done
         }
     }
 
     pub fn wait_build(&self, build_url: &str) -> JenkinsBuildStatus {
         // TODO: Implement a timeout?
-        loop {
-            match self.get_build_status(&build_url) {
-                JenkinsBuildStatus::Done => return JenkinsBuildStatus::Done,
-                _ => { },
-            }
+        while self.get_build_status(build_url) != JenkinsBuildStatus::Done {
             sleep(Duration::from_millis(JENKINS_POLLING_INTERVAL));
         }
+        JenkinsBuildStatus::Done
     }
 }
diff --git a/src/main.rs b/src/main.rs
index df3ff03..21c8dd5 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -103,7 +103,7 @@  fn run_tests(settings: &Config, client: Arc<Client>, project: &Project, tag: &st
         token: settings.jenkins.token.clone(),
     };
     let project = project.clone();
-    for job_params in project.jobs.iter() {
+    for job_params in &project.jobs {
         let job_name = job_params.get("job").unwrap();
         let mut jenkins_params = Vec::<(&str, &str)>::new();
         for (param_name, param_value) in job_params.iter() {
@@ -111,21 +111,21 @@  fn run_tests(settings: &Config, client: Arc<Client>, project: &Project, tag: &st
             match param_name.as_ref() {
                 // TODO: Validate special parameter names in config at start of program
                 "job" => { },
-                "remote" => jenkins_params.push((&param_value, &project.remote_uri)),
-                "branch" => jenkins_params.push((&param_value, &tag)),
-                _ => jenkins_params.push((&param_name, &param_value)),
+                "remote" => jenkins_params.push((param_value, &project.remote_uri)),
+                "branch" => jenkins_params.push((param_value, tag)),
+                _ => jenkins_params.push((param_name, param_value)),
             }
         }
         info!("Starting job: {}", &job_name);
-        let res = jenkins.start_test(&job_name, jenkins_params)
+        let res = jenkins.start_test(job_name, jenkins_params)
             .unwrap_or_else(|err| panic!("Starting Jenkins test failed: {}", err));
         debug!("{:?}", &res);
         let build_url_real;
         loop {
             let build_url = jenkins.get_build_url(&res);
-            match build_url {
-                Some(url) => { build_url_real = url; break; },
-                None => { },
+            if let Some(url) = build_url {
+                build_url_real = url;
+                break;
             }
         }
         debug!("Build URL: {}", build_url_real);
@@ -154,7 +154,7 @@  fn test_patch(settings: &Config, client: &Arc<Client>, project: &Project, path:
 
     let mut push_callbacks = RemoteCallbacks::new();
     push_callbacks.credentials(|_, _, _| {
-        return Cred::ssh_key_from_agent("git");
+        Cred::ssh_key_from_agent("git")
     });
 
     let mut push_opts = PushOptions::new();
@@ -179,12 +179,10 @@  fn test_patch(settings: &Config, client: &Arc<Client>, project: &Project, path:
             .unwrap_or_else(|err| panic!("Couldn't checkout HEAD: {}", err));
         debug!("Repo is now at head {}", repo.head().unwrap().name().unwrap());
 
-        let output = git::apply_patch(&repo, &path);
+        let output = git::apply_patch(&repo, path);
 
-        match output {
-            Ok(_) => git::push_to_remote(
-                &mut remote, &tag, &mut push_opts).unwrap(),
-            _ => {}
+        if output.is_ok() {
+            git::push_to_remote(&mut remote, &tag, &mut push_opts).unwrap();
         }
 
         git::checkout_branch(&repo, &branch_name);
@@ -223,8 +221,7 @@  fn test_patch(settings: &Config, client: &Arc<Client>, project: &Project, path:
 
         // We've set up a remote branch, time to kick off tests
         let test = thread::Builder::new().name(tag.to_string()).spawn(move || {
-            return run_tests(&settings_clone, client, &project, &tag,
-                             &branch_name);
+            run_tests(&settings_clone, client, &project, &tag, &branch_name)
         }).unwrap();
         results.append(&mut test.join().unwrap());
 
@@ -242,6 +239,7 @@  fn test_patch(settings: &Config, client: &Arc<Client>, project: &Project, path:
     results
 }
 
+#[cfg_attr(feature="clippy", allow(cyclomatic_complexity))]
 fn main() {
     let mut log_builder = LogBuilder::new();
     // By default, log at the "info" level for every module
@@ -296,7 +294,7 @@  fn main() {
         match settings.projects.get(&args.flag_project) {
             None => panic!("Couldn't find project {}", args.flag_project),
             Some(project) => {
-                test_patch(&settings, &client, &project, &patch);
+                test_patch(&settings, &client, project, patch);
             }
         }
 
@@ -310,7 +308,7 @@  fn main() {
             None => panic!("Couldn't find project {}", &series.project.linkname),
             Some(project) => {
                 let patch = patchwork.get_patch(&series);
-                test_patch(&settings, &client, &project, &patch);
+                test_patch(&settings, &client, project, &patch);
             }
         }
 
@@ -346,7 +344,7 @@  fn main() {
                 },
                 Some(project) => {
                     let patch = patchwork.get_patch(&series);
-                    let results = test_patch(&settings, &client, &project, &patch);
+                    let results = test_patch(&settings, &client, project, &patch);
                     // Delete the temporary directory with the patch in it
                     fs::remove_dir_all(patch.parent().unwrap()).unwrap_or_else(
                         |err| error!("Couldn't delete temp directory: {}", err));
diff --git a/src/patchwork.rs b/src/patchwork.rs
index 39ff81b..b11f6ac 100644
--- a/src/patchwork.rs
+++ b/src/patchwork.rs
@@ -116,6 +116,7 @@  pub struct PatchworkServer {
 }
 
 impl PatchworkServer {
+    #[cfg_attr(feature="clippy", allow(ptr_arg))]
     pub fn new(url: &String, client: &std::sync::Arc<Client>) -> PatchworkServer {
         let mut headers = Headers::new();
         headers.set(Accept(vec![qitem(Mime(TopLevel::Application,
@@ -133,6 +134,7 @@  impl PatchworkServer {
         }
     }
 
+    #[cfg_attr(feature="clippy", allow(ptr_arg))]
     pub fn set_authentication(&mut self, username: &String, password: &Option<String>) {
         self.headers.set(Authorization(Basic {
             username: username.clone(),
diff --git a/src/settings.rs b/src/settings.rs
index a4a5614..4e91244 100644
--- a/src/settings.rs
+++ b/src/settings.rs
@@ -59,7 +59,7 @@  pub struct Project {
 
 impl Project {
     pub fn get_repo(&self) -> Result<Repository, Error> {
-        return Repository::open(&self.repository);
+        Repository::open(&self.repository)
     }
 }