From patchwork Thu Jan 5 05:53:36 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Donnellan X-Patchwork-Id: 711247 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3tvH1J4mxFz9rxw for ; Thu, 5 Jan 2017 16:53:56 +1100 (AEDT) Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 3tvH1J3TDhzDqN4 for ; Thu, 5 Jan 2017 16:53:56 +1100 (AEDT) X-Original-To: snowpatch@lists.ozlabs.org Delivered-To: snowpatch@lists.ozlabs.org Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3tvH1C3cQjzDqMh for ; Thu, 5 Jan 2017 16:53:50 +1100 (AEDT) Received: from pps.filterd (m0098414.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id v055rfNf031310 for ; Thu, 5 Jan 2017 00:53:48 -0500 Received: from e23smtp02.au.ibm.com (e23smtp02.au.ibm.com [202.81.31.144]) by mx0b-001b2d01.pphosted.com with ESMTP id 27sg8k8926-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 05 Jan 2017 00:53:48 -0500 Received: from localhost by e23smtp02.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 5 Jan 2017 15:53:45 +1000 Received: from d23dlp03.au.ibm.com (202.81.31.214) by e23smtp02.au.ibm.com (202.81.31.208) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Thu, 5 Jan 2017 15:53:42 +1000 Received: from d23relay08.au.ibm.com (d23relay08.au.ibm.com [9.185.71.33]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id ACFB33578057 for ; Thu, 5 Jan 2017 16:53:41 +1100 (EST) Received: from d23av06.au.ibm.com (d23av06.au.ibm.com [9.190.235.151]) by d23relay08.au.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v055rhql54591488 for ; Thu, 5 Jan 2017 16:53:43 +1100 Received: from d23av06.au.ibm.com (localhost [127.0.0.1]) by d23av06.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id v055rfrD001255 for ; Thu, 5 Jan 2017 16:53:41 +1100 Received: from ozlabs.au.ibm.com (ozlabs.au.ibm.com [9.192.253.14]) by d23av06.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id v055rfaC001252 for ; Thu, 5 Jan 2017 16:53:41 +1100 Received: from ajd.ozlabs.ibm.com (haven.au.ibm.com [9.192.254.114]) (using TLSv1.2 with cipher AES128-SHA256 (128/128 bits)) (No client certificate requested) by ozlabs.au.ibm.com (Postfix) with ESMTPSA id DE0B3A01AB for ; Thu, 5 Jan 2017 16:53:40 +1100 (AEDT) From: Andrew Donnellan To: snowpatch@lists.ozlabs.org Date: Thu, 5 Jan 2017 16:53:36 +1100 X-Mailer: git-send-email 2.11.0 X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 17010505-0004-0000-0000-000001D053C5 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17010505-0005-0000-0000-0000096EA029 Message-Id: <20170105055336.30018-1-andrew.donnellan@au1.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-01-05_03:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=1 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1612050000 definitions=main-1701050096 Subject: [snowpatch] [PATCH v3] Add documentation on clippy, fix clippy warnings X-BeenThere: snowpatch@lists.ozlabs.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Continuous Integration for patch-based workflows List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: snowpatch-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "snowpatch" clippy[0] is the de facto standard linter for Rust. Add documentation explaining how to run clippy on a nightly toolchain. Fix the warnings it gives us and add a few exceptions where reasonable. [0] https://github.com/Manishearth/rust-clippy Signed-off-by: Andrew Donnellan --- v2->v3: * get rid of the feature stuff, just tell people to use cargo +nightly clippy --- CONTRIBUTING.md | 7 ++++++- src/git.rs | 10 +++++----- src/jenkins.rs | 15 +++++++-------- src/main.rs | 36 +++++++++++++++++------------------- src/patchwork.rs | 2 ++ src/settings.rs | 2 +- 6 files changed, 38 insertions(+), 34 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d067da9..8d04072 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,6 +28,11 @@ We like to follow the [Rust Guidelines](https://aturon.github.io/) where possible - patches to fix existing non-compliant code are welcome! +If you're using a nightly Rust toolchain, you can use the +[clippy](https://github.com/Manishearth/rust-clippy) linter: `cargo ++nightly install clippy` to install, and `cargo +nightly clippy` to +run. + When your patch involves creating a new file, where possible please use a header along the lines of: @@ -46,4 +51,4 @@ use a header along the lines of: # # [FILENAME] - [DESCRIPTION] # -``` \ No newline at end of file +``` 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 { 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 426bfdf..e4d2389 100644 --- a/src/main.rs +++ b/src/main.rs @@ -99,7 +99,7 @@ fn run_tests(settings: &Config, client: Arc, 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() { @@ -107,21 +107,21 @@ fn run_tests(settings: &Config, client: Arc, 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((¶m_value, &project.remote_uri)), - "branch" => jenkins_params.push((¶m_value, &tag)), - _ => jenkins_params.push((¶m_name, ¶m_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); @@ -150,7 +150,7 @@ fn test_patch(settings: &Config, client: &Arc, 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(); @@ -175,12 +175,10 @@ fn test_patch(settings: &Config, client: &Arc, 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); @@ -219,8 +217,7 @@ fn test_patch(settings: &Config, client: &Arc, 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()); @@ -238,6 +235,7 @@ fn test_patch(settings: &Config, client: &Arc, project: &Project, path: results } +#[cfg_attr(feature="cargo-clippy", allow(cyclomatic_complexity))] fn main() { let mut log_builder = LogBuilder::new(); // By default, log at the "info" level for every module @@ -292,7 +290,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); } } @@ -306,7 +304,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); } } @@ -342,7 +340,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 fa36da9..a6f761a 100644 --- a/src/patchwork.rs +++ b/src/patchwork.rs @@ -116,6 +116,7 @@ pub struct PatchworkServer { } impl PatchworkServer { + #[cfg_attr(feature="cargo-clippy", allow(ptr_arg))] pub fn new(url: &String, client: &std::sync::Arc) -> PatchworkServer { let mut headers = Headers::new(); headers.set(Accept(vec![qitem(Mime(TopLevel::Application, @@ -133,6 +134,7 @@ impl PatchworkServer { } } + #[cfg_attr(feature="cargo-clippy", allow(ptr_arg))] pub fn set_authentication(&mut self, username: &String, password: &Option) { 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 { - return Repository::open(&self.repository); + Repository::open(&self.repository) } }