From 207f6c13934f66e54ccc68b8e47b2a529e591e88 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Tue, 24 Dec 2024 18:32:38 +0100 Subject: [PATCH] refactor(vcs/generic): introduce ChangeStatus and remove CommitStatus CommitStatus was too rigid for GitHub legacy usecases. It is replaced by a more flexible ChangeStatus tailored for Gerrit richer features. Signed-off-by: Raito Bezarius --- ofborg/src/evalchecker.rs | 45 +++++- ofborg/src/tasks/eval/nixpkgs.rs | 165 ++++++++++++++-------- ofborg/src/tasks/evaluate.rs | 158 ++++++++++----------- ofborg/src/vcs/generic/change_status.rs | 176 ++++++++++++++++++++++++ ofborg/src/vcs/generic/checks.rs | 16 ++- ofborg/src/vcs/generic/commit_status.rs | 85 ------------ ofborg/src/vcs/generic/mod.rs | 2 + 7 files changed, 414 insertions(+), 233 deletions(-) create mode 100644 ofborg/src/vcs/generic/change_status.rs delete mode 100644 ofborg/src/vcs/generic/commit_status.rs diff --git a/ofborg/src/evalchecker.rs b/ofborg/src/evalchecker.rs index e5865cb..8004328 100644 --- a/ofborg/src/evalchecker.rs +++ b/ofborg/src/evalchecker.rs @@ -1,10 +1,12 @@ use crate::nix; +use crate::vcs::generic::{Category, CheckResult, Link}; use std::fs::File; use std::path::Path; pub struct EvalChecker { name: String, + failure_category: Category, op: nix::Operation, args: Vec, nix: nix::Nix, @@ -12,9 +14,16 @@ pub struct EvalChecker { impl EvalChecker { #[must_use] - pub fn new(name: &str, op: nix::Operation, args: Vec, nix: nix::Nix) -> EvalChecker { + pub fn new( + name: &str, + failure_category: Category, + op: nix::Operation, + args: Vec, + nix: nix::Nix, + ) -> EvalChecker { EvalChecker { name: name.to_owned(), + failure_category, op, args, nix, @@ -26,6 +35,16 @@ impl EvalChecker { &self.name } + #[must_use] + pub fn success_category(&self) -> Category { + Category::Success + } + + #[must_use] + pub fn failure_category(&self) -> Category { + self.failure_category + } + pub async fn execute(&self, path: &Path) -> Result { self.nix .safely(&self.op, path, self.args.clone(), false) @@ -38,4 +57,28 @@ impl EvalChecker { cli.append(&mut self.args.clone()); cli.join(" ") } + + pub fn into_successful_result(&self) -> CheckResult { + CheckResult { + external_id: None, + category: self.success_category(), + summary: self.cli_cmd(), + message: None, + tags: vec![], + links: vec![], + code_pointers: vec![], + } + } + + pub fn into_failed_result(&self, links: Vec) -> CheckResult { + CheckResult { + external_id: None, + category: self.failure_category(), + summary: self.cli_cmd(), + message: None, + tags: vec![], + links, + code_pointers: vec![], + } + } } diff --git a/ofborg/src/tasks/eval/nixpkgs.rs b/ofborg/src/tasks/eval/nixpkgs.rs index 210617f..7203317 100644 --- a/ofborg/src/tasks/eval/nixpkgs.rs +++ b/ofborg/src/tasks/eval/nixpkgs.rs @@ -11,7 +11,7 @@ use crate::outpathdiff::{OutPathDiff, PackageArch}; use crate::tagger::{MaintainerPrTagger, PkgsAddedRemovedTagger, RebuildTagger, StdenvTagger}; use crate::tasks::eval::{stdenvs::Stdenvs, Error, EvaluationComplete, StepResult}; use crate::vcs::generic::{ - CheckResult, CheckRun, CheckRunState, CommitStatus, VersionControlSystemAPI, + Category, ChangeStatus, CheckResult, CheckRun, CheckRunState, VersionControlSystemAPI, }; use std::path::Path; @@ -249,14 +249,16 @@ impl<'a> NixpkgsStrategy<'a> { async fn update_rebuild_labels( &mut self, dir: &Path, - overall_status: &mut CommitStatus, + overall_status: &mut ChangeStatus, ) -> Result<(), Error> { if let Some(ref rebuildsniff) = self.outpath_diff { let mut rebuild_tags = RebuildTagger::new(); if let Some(attrs) = rebuildsniff.calculate_rebuild() { if !attrs.is_empty() { - overall_status.set_url(self.gist_changed_paths(&attrs).await); + overall_status + .set_status_link(self.gist_changed_paths(&attrs).await) + .await; self.record_impacted_maintainers(dir, &attrs).await?; } @@ -295,6 +297,15 @@ impl<'a> NixpkgsStrategy<'a> { .collect::>>(); if let Some(ref changed_paths) = self.changed_paths { + let mut status = ChangeStatus::builder(&self.job.change) + .name("Automatic maintainer request") + .description("This check will evaluate automatically impacted maintainers by this change and request them. In case of too large changes, this step is skipped.") + .build(); + + status + .create(self.vcs_api.clone(), CheckRunState::Running) + .await; + let m = ImpactedMaintainers::calculate(&self.nix, dir, changed_paths, &changed_attributes) .await; @@ -316,27 +327,24 @@ impl<'a> NixpkgsStrategy<'a> { "pull request has {} changed paths, skipping review requests", changed_paths.len() ); - let status = CommitStatus::new( - self.vcs_api.clone(), - self.repo.clone(), - self.job.change.head_sha.clone(), - "ofborg-eval-check-maintainers".to_owned(), - String::from("large change, skipping automatic review requests"), - gist_url, - ); - status.set(CheckRunState::Completed).await?; + + status + .update_status_with_description( + "large change, skipping automatic review requests", + CheckRunState::Completed, + gist_url, + ) + .await; return Ok(()); } - let status = CommitStatus::new( - self.vcs_api.clone(), - self.repo.clone(), - self.job.change.head_sha.clone(), - "ofborg-eval-check-maintainers".to_owned(), - String::from("matching changed paths to changed attrs..."), - gist_url, - ); - status.set(CheckRunState::Completed).await?; + status + .update_status_with_description( + "matching changed paths to changed attrs...", + CheckRunState::Completed, + gist_url, + ) + .await; if let Ok(ref maint) = m { self.request_reviews(maint).await; @@ -359,15 +367,13 @@ impl<'a> NixpkgsStrategy<'a> { async fn check_meta_queue_builds(&mut self, dir: &Path) -> StepResult> { if let Some(ref possibly_touched_packages) = self.touched_packages { - let mut status = CommitStatus::new( - self.vcs_api.clone(), - self.repo.clone(), - self.job.change.head_sha.clone(), - "ci-eval-check-meta".to_owned(), - String::from("config.nix: checkMeta = true"), - None, - ); - status.set(CheckRunState::Completed).await?; + let status = ChangeStatus::builder(&self.job.change) + .name("Evaluate with meta attributes typing") + .description("This runs a nixpkgs evaluation with `config.checkMeta = true;` during nixpkgs instantiation") + .label_name("Verified") + .build(); + + status.update_status(CheckRunState::Running).await; let nixenv = HydraNixEnv::new(self.nix.clone(), dir.to_path_buf(), true); match nixenv.execute_with_stats().await { @@ -381,8 +387,7 @@ impl<'a> NixpkgsStrategy<'a> { try_build.sort(); try_build.dedup(); - status.set_url(None); - status.set(CheckRunState::Completed).await?; + status.update_status(CheckRunState::Completed).await; if !try_build.is_empty() && try_build.len() <= 20 { // In the case of trying to merge master in to @@ -403,17 +408,21 @@ impl<'a> NixpkgsStrategy<'a> { } } Err(out) => { - status.set_url( - crate::utils::pastebin::make_pastebin( - &mut self.chan, - "Meta Check", - out.display(), + status + .update_status_with_description( + "Meta check failed", + CheckRunState::Completed, + crate::utils::pastebin::make_pastebin( + &mut self.chan, + "Meta Check", + out.display(), + ) + .await + .ok() + .map(|pp| pp.uri), ) - .await - .ok() - .map(|pp| pp.uri), - ); - // status.set(State::Failure).await?; + .await; + // TODO: add a failed result with the details. Err(Error::Fail(String::from( "Failed to validate package metadata.", ))) @@ -429,19 +438,47 @@ impl<'a> NixpkgsStrategy<'a> { Ok(()) } + pub(crate) async fn preflight_check( + &self, + target_branch: &str, + status: &mut ChangeStatus, + ) -> StepResult { + if target_branch.starts_with("nixos-") || target_branch.starts_with("nixpkgs-") { + status + .update_description( + "The branch you have targeted is a read-only mirror for channels. \ + Please target release-* or master.", + ) + .await; + + info!("PR targets a nixos-* or nixpkgs-* branch"); + return Ok(false); + }; + + Ok(true) + } + pub(crate) async fn on_target_branch( &mut self, dir: &Path, - status: &mut CommitStatus, + status: &mut ChangeStatus, ) -> StepResult<()> { status - .set_with_description("Checking original stdenvs", CheckRunState::Scheduled) - .await?; + .update_status_with_description( + "Checking original stdenvs", + CheckRunState::Scheduled, + None, + ) + .await; self.check_stdenvs_before(dir).await; status - .set_with_description("Checking original out paths", CheckRunState::Scheduled) - .await?; + .update_status_with_description( + "Checking original out paths", + CheckRunState::Scheduled, + None, + ) + .await; self.check_outpaths_before(dir).await?; Ok(()) @@ -464,18 +501,22 @@ impl<'a> NixpkgsStrategy<'a> { .await; } - pub(crate) async fn after_merge(&mut self, status: &mut CommitStatus) -> StepResult<()> { + pub(crate) async fn after_merge(&mut self, status: &mut ChangeStatus) -> StepResult<()> { self.update_labels(&[], &["2.status: merge conflict".to_owned()]) .await; status - .set_with_description("Checking new stdenvs", CheckRunState::Scheduled) - .await?; + .update_status_with_description("Checking new stdenvs", CheckRunState::Scheduled, None) + .await; self.check_stdenvs_after().await; status - .set_with_description("Checking new out paths", CheckRunState::Scheduled) - .await?; + .update_status_with_description( + "Checking new out paths", + CheckRunState::Scheduled, + None, + ) + .await; self.check_outpaths_after().await?; Ok(()) @@ -492,12 +533,14 @@ impl<'a> NixpkgsStrategy<'a> { vec![ EvalChecker::new( "package-list", + Category::Error, nix::Operation::QueryPackagesJson, vec![String::from("--file"), String::from(".")], self.nix.clone(), ), EvalChecker::new( "package-list-with-aliases", + Category::Error, nix::Operation::QueryPackagesJson, vec![ String::from("--file"), @@ -510,6 +553,7 @@ impl<'a> NixpkgsStrategy<'a> { ), EvalChecker::new( "lib-tests", + Category::Error, nix::Operation::Build, vec![ String::from("--arg"), @@ -521,6 +565,7 @@ impl<'a> NixpkgsStrategy<'a> { ), EvalChecker::new( "nixos", + Category::Error, nix::Operation::Instantiate, vec![ String::from("--arg"), @@ -534,6 +579,7 @@ impl<'a> NixpkgsStrategy<'a> { ), EvalChecker::new( "nixos-options", + Category::Error, nix::Operation::Instantiate, vec![ String::from("--arg"), @@ -547,6 +593,7 @@ impl<'a> NixpkgsStrategy<'a> { ), EvalChecker::new( "nixos-manual", + Category::Error, nix::Operation::Instantiate, vec![ String::from("--arg"), @@ -560,6 +607,7 @@ impl<'a> NixpkgsStrategy<'a> { ), EvalChecker::new( "nixpkgs-manual", + Category::Error, nix::Operation::Instantiate, vec![ String::from("--arg"), @@ -573,6 +621,7 @@ impl<'a> NixpkgsStrategy<'a> { ), EvalChecker::new( "nixpkgs-tarball", + Category::Error, nix::Operation::Instantiate, vec![ String::from("--arg"), @@ -586,6 +635,7 @@ impl<'a> NixpkgsStrategy<'a> { ), EvalChecker::new( "nixpkgs-unstable-jobset", + Category::Error, nix::Operation::Instantiate, vec![ String::from("--arg"), @@ -599,6 +649,7 @@ impl<'a> NixpkgsStrategy<'a> { ), EvalChecker::new( "darwin", + Category::Warning, nix::Operation::Instantiate, vec![ String::from("--arg"), @@ -616,13 +667,17 @@ impl<'a> NixpkgsStrategy<'a> { pub(crate) async fn all_evaluations_passed( &mut self, dir: &Path, - status: &mut CommitStatus, + status: &mut ChangeStatus, ) -> StepResult { self.update_stdenv_labels().await; status - .set_with_description("Calculating Changed Outputs", CheckRunState::Scheduled) - .await?; + .update_status_with_description( + "Calculating Changed Outputs", + CheckRunState::Scheduled, + None, + ) + .await; self.update_new_package_labels().await; self.update_rebuild_labels(dir, status).await?; diff --git a/ofborg/src/tasks/evaluate.rs b/ofborg/src/tasks/evaluate.rs index ae48620..6479885 100644 --- a/ofborg/src/tasks/evaluate.rs +++ b/ofborg/src/tasks/evaluate.rs @@ -10,8 +10,8 @@ use crate::systems; use crate::tasks::eval; use crate::utils::pastebin::PersistedPastebin; use crate::vcs::generic::{ - AugmentedVCSApi, CheckRunState, CommitStatus, CommitStatusError, Issue, IssueState, - VersionControlSystemAPI, + AugmentedVCSApi, ChangeStatus, CheckResult, CheckRunState, CommitStatusError, Issue, + IssueState, Link, VersionControlSystemAPI, }; use crate::vcs::gerrit::http::GerritHTTPApi; use crate::worker; @@ -124,6 +124,7 @@ struct OneEval<'a, E> { identity: &'a str, cloner: &'a checkout::CachedCloner, job: &'a evaluationjob::EvaluationJob, + status: ChangeStatus, } impl<'a, E: stats::SysEvents + 'static> OneEval<'a, E> { @@ -145,6 +146,10 @@ impl<'a, E: stats::SysEvents + 'static> OneEval<'a, E> { identity, cloner, job, + status: ChangeStatus::builder(&job.change) + .name("Nix evaluation") + .description("Run a Nix-based evaluation strategy on this change") + .build(), } } @@ -305,18 +310,7 @@ impl<'a, E: stats::SysEvents + 'static> OneEval<'a, E> { self.nix.clone(), ); - let mut overall_status = CommitStatus::new( - self.vcs_api.clone(), - job.repo.clone(), - job.change.head_sha.clone(), - "ofborg-eval".to_owned(), - "Starting".to_owned(), - None, - ); - - overall_status - .set_with_description("Starting", CheckRunState::Running) - .await?; + self.status.update_description("Pre-cloning").await; evaluation_strategy.pre_clone().await?; @@ -324,9 +318,7 @@ impl<'a, E: stats::SysEvents + 'static> OneEval<'a, E> { .cloner .project(&job.repo.full_name, job.repo.clone_url.clone()); - overall_status - .set_with_description("Cloning project", CheckRunState::Running) - .await?; + self.status.update_description("Cloning project").await; info!("Working on {}", job.change.number); let co = project @@ -339,31 +331,26 @@ impl<'a, E: stats::SysEvents + 'static> OneEval<'a, E> { None => String::from("master"), }; - // TODO: this is a preflight check, encode it as such. - if target_branch.starts_with("nixos-") || target_branch.starts_with("nixpkgs-") { - overall_status - .set_with_description( - "The branch you have targeted is a read-only mirror for channels. \ - Please target release-* or master.", - CheckRunState::Completed, - ) - .await?; + self.status.update_description("Pre-flight checking").await; - info!("PR targets a nixos-* or nixpkgs-* branch"); + if !evaluation_strategy + .preflight_check(&target_branch, &mut self.status) + .await? + { + self.status.update_status(CheckRunState::Completed).await; + info!("Pre-flight check failed, skipping this job"); return Ok(Actions::skip(job)); - }; + } + + self.status + .update_description(format!("Checking out {}", &target_branch).as_ref()) + .await; - overall_status - .set_with_description( - format!("Checking out {}", &target_branch).as_ref(), - CheckRunState::Running, - ) - .await?; info!("Checking out target branch {}", &target_branch); let refpath = co.checkout_origin_ref(target_branch.as_ref()).unwrap(); evaluation_strategy - .on_target_branch(Path::new(&refpath), &mut overall_status) + .on_target_branch(Path::new(&refpath), &mut self.status) .await?; let target_branch_rebuild_sniff_start = Instant::now(); @@ -375,19 +362,21 @@ impl<'a, E: stats::SysEvents + 'static> OneEval<'a, E> { )) .await; self.events - .notify(Event::EvaluationDurationCount(target_branch)) + .notify(Event::EvaluationDurationCount(target_branch.clone())) .await; - overall_status - .set_with_description("Fetching PR", CheckRunState::Running) - .await?; + self.status.update_description("Fetching change").await; co.fetch_change(&job.change).unwrap(); if !co.commit_exists(job.change.head_sha.as_ref()) { - overall_status - .set_with_description("Commit not found", CheckRunState::Running) - .await?; + self.status + .update_status_with_description( + "Change's commit not found", + CheckRunState::Completed, + None, + ) + .await; info!("Commit {} doesn't exist", job.change.head_sha); return Ok(Actions::skip(job)); @@ -395,14 +384,16 @@ impl<'a, E: stats::SysEvents + 'static> OneEval<'a, E> { evaluation_strategy.after_fetch(&co); - overall_status - .set_with_description("Merging PR", CheckRunState::Running) - .await?; + self.status + .update_description( + format!("Merging change in target branch {}", target_branch).as_ref(), + ) + .await; if co.merge_commit(job.change.head_sha.as_ref()).is_err() { - overall_status - .set_with_description("Failed to merge", CheckRunState::Running) - .await?; + self.status + .update_description("Failed to merge; executing merge conflict strategy") + .await; info!("Failed to merge {}", job.change.head_sha); @@ -411,58 +402,47 @@ impl<'a, E: stats::SysEvents + 'static> OneEval<'a, E> { return Ok(Actions::skip(job)); } - evaluation_strategy.after_merge(&mut overall_status).await?; + evaluation_strategy.after_merge(&mut self.status).await?; info!("Got path: {:?}, building", refpath); - overall_status - .set_with_description("Beginning Evaluations", CheckRunState::Running) - .await?; + self.status + .update_description("Beginning Evaluations") + .await; let mut all_good = true; + // TODO: conduct all checks in parallel and send them as soon as they are available. for check in evaluation_strategy.evaluation_checks() { - let mut status = CommitStatus::new( - self.vcs_api.clone(), - job.repo.clone(), - job.change.head_sha.clone(), - format!("ofborg-eval-{}", check.name()), - check.cli_cmd(), - None, - ); - - status - .set(CheckRunState::Running) - .await - .expect("Failed to set status on eval strategy"); - - let state: CheckRunState; - let gist_url: Option; + let result: CheckResult; match check.execute(Path::new(&refpath)).await { Ok(_) => { - state = CheckRunState::Completed; - gist_url = None; + result = check.into_successful_result(); } Err(mut out) => { - state = CheckRunState::Completed; - gist_url = self + let gist_link = self .make_pastebin( chan, &format!("[ofborg] Evaluation of {}", check.name()), file_to_str(&mut out), ) .await - .map(|pp| pp.uri); + .map(|pp| pp.uri) + .map(|url| Link { + url, + tooltip: Some("Details of this evaluation check".to_owned()), + primary: true, + icon: crate::vcs::generic::LinkIcon::History, + }); + + result = + check.into_failed_result(vec![gist_link].into_iter().flatten().collect()); } } - status.set_url(gist_url); - status - .set(state) - .await - .expect("Failed to set status on eval strategy"); - - if state != CheckRunState::Completed { + if !result.is_successful() { all_good = false; } + + self.status.add_result(result).send_results().await; } info!("Finished evaluations"); @@ -470,7 +450,7 @@ impl<'a, E: stats::SysEvents + 'static> OneEval<'a, E> { if all_good { let complete = evaluation_strategy - .all_evaluations_passed(Path::new(&refpath), &mut overall_status) + .all_evaluations_passed(Path::new(&refpath), &mut self.status) .await?; self.vcs_api @@ -478,13 +458,17 @@ impl<'a, E: stats::SysEvents + 'static> OneEval<'a, E> { .await; response.extend(schedule_builds(complete.builds, &auto_schedule_build_archs)); - overall_status - .set_with_description("^.^!", CheckRunState::Completed) - .await?; + self.status + .update_status_with_description("^.^!", CheckRunState::Completed, None) + .await; } else { - overall_status - .set_with_description("Complete, with errors", CheckRunState::Completed) - .await?; + self.status + .update_status_with_description( + "Complete, with errors", + CheckRunState::Completed, + None, + ) + .await; } self.events.notify(Event::TaskEvaluationCheckComplete).await; diff --git a/ofborg/src/vcs/generic/change_status.rs b/ofborg/src/vcs/generic/change_status.rs new file mode 100644 index 0000000..5a28ece --- /dev/null +++ b/ofborg/src/vcs/generic/change_status.rs @@ -0,0 +1,176 @@ +/// Change status is an evolution of the legacy CommitStatus control structure +/// which was really only tailored for simple usecases and GitHub. +use std::sync::Arc; + +use chrono::NaiveDateTime; + +use crate::message::Change; + +use super::{CheckResult, CheckRunState, VersionControlSystemAPI}; + +/// This is a structure to control a specific check run and its results. +pub struct ChangeStatus { + // Internal information for the status server. + change: u64, + patchset: u64, + attempt: u64, + + /// Global name of this check. Must be unique on a given change. + name: String, + /// Description of what this check does. + description: String, + /// A link to documentation about what does this specific check. + doc_link: Option, + /// What label does this check affect? To help causality analysis on the frontend. + label_name: Option, + /// Scheduling timestamp + scheduled_timestamp: Option, + /// Started timestamp + started_timestamp: Option, + /// Estimated finished timestamp or actual finished timestamp. + finished_timestamp: Option, + + status: CheckRunState, + status_link: Option, + results: Vec, +} + +impl ChangeStatus {} + +/// Builder for ChangeStatus. +pub struct ChangeStatusBuilder { + change: u64, + patchset: u64, + attempt: u64, + name: String, + description: String, + doc_link: Option, + status: CheckRunState, + status_link: Option, + label_name: Option, + scheduled_timestamp: Option, + finished_timestamp: Option, + started_timestamp: Option, + results: Vec, +} + +impl ChangeStatusBuilder { + pub fn name(mut self, name: &str) -> Self { + self.name = name.to_owned(); + self + } + + pub fn description(mut self, description: &str) -> Self { + self.description = description.to_owned(); + self + } + + pub fn doc_link(mut self, doc_link: Option) -> Self { + self.doc_link = doc_link; + self + } + + pub fn label_name(mut self, label_name: &str) -> Self { + self.label_name = Some(label_name.to_owned()); + self + } + + pub fn scheduled_timestamp(mut self, timestamp: Option) -> Self { + self.scheduled_timestamp = timestamp; + self + } + + pub fn finished_timestamp(mut self, timestamp: Option) -> Self { + self.finished_timestamp = timestamp; + self + } + + pub fn build(self) -> ChangeStatus { + ChangeStatus { + change: self.change, + patchset: self.patchset, + attempt: self.attempt, + name: self.name, + description: self.description, + doc_link: self.doc_link, + status: self.status, + status_link: self.status_link, + label_name: self.label_name, + scheduled_timestamp: self.scheduled_timestamp, + finished_timestamp: self.finished_timestamp, + started_timestamp: self.started_timestamp, + results: self.results, + } + } +} + +impl ChangeStatus { + /// Create a new builder instance. + pub fn builder(change: &Change) -> ChangeStatusBuilder { + ChangeStatusBuilder { + change: change.number, + patchset: 0, + attempt: 0, + name: String::new(), + description: String::new(), + doc_link: None, + status: CheckRunState::Runnable, + status_link: None, + label_name: None, + scheduled_timestamp: None, + finished_timestamp: None, + started_timestamp: None, + results: Vec::new(), + } + } + + /// This creates the change status over the API, making it possibly visible to VCSes. + pub async fn create( + &mut self, + api: Arc, + initial_state: CheckRunState, + ) -> Self { + self.status = initial_state; + todo!(); + } + + pub async fn set_started(&self) { + // Update the started timestamp. + todo!(); + } + + pub async fn update_description(&self, description: &str) { + todo!(); + } + + /// This updates the current status of this check with a description. + pub async fn update_status_with_description( + &self, + description: &str, + status: CheckRunState, + link: Option, + ) { + todo!(); + } + + pub async fn set_status_link(&mut self, link: Option) { + self.status_link = link; + todo!(); + } + + pub async fn update_status(&self, status: CheckRunState) { + todo!(); + } + + /// Add a result regarding this check, it does not get sent immediately. + /// Call [`send_results`] to send it. + pub fn add_result(&mut self, result: CheckResult) -> &mut Self { + self.results.push(result); + self + } + + /// This sends the results via the API and make them visible to servers. + pub async fn send_results(&mut self) -> Self { + todo!(); + } +} diff --git a/ofborg/src/vcs/generic/checks.rs b/ofborg/src/vcs/generic/checks.rs index 6c9d0d2..7ed4922 100644 --- a/ofborg/src/vcs/generic/checks.rs +++ b/ofborg/src/vcs/generic/checks.rs @@ -66,7 +66,13 @@ pub struct CheckResult { pub code_pointers: Vec, } -#[derive(Debug, Serialize, Deserialize, PartialEq)] +impl CheckResult { + pub fn is_successful(&self) -> bool { + self.category == Category::Success + } +} + +#[derive(Debug, Serialize, Deserialize, PartialEq, Clone, Copy)] #[serde(rename_all = "UPPERCASE")] pub enum Category { Success, @@ -97,11 +103,11 @@ pub struct Tag { #[derive(Debug, Serialize, Deserialize, PartialEq)] pub struct Link { - url: String, + pub url: String, #[serde(skip_serializing_if = "Option::is_none")] - tooltip: Option, - primary: bool, - icon: LinkIcon, + pub tooltip: Option, + pub primary: bool, + pub icon: LinkIcon, } #[derive(Debug, Serialize, Deserialize, PartialEq)] diff --git a/ofborg/src/vcs/generic/commit_status.rs b/ofborg/src/vcs/generic/commit_status.rs deleted file mode 100644 index 8776677..0000000 --- a/ofborg/src/vcs/generic/commit_status.rs +++ /dev/null @@ -1,85 +0,0 @@ -use std::sync::Arc; - -use tracing::warn; - -use super::{CheckRunState, VersionControlSystemAPI}; - -pub struct CommitStatus { - api: Arc, - repo: crate::message::Repo, - sha: String, - context: String, - description: String, - url: String, -} - -impl CommitStatus { - pub fn new( - api: Arc, - repo: crate::message::Repo, - sha: String, - context: String, - description: String, - url: Option, - ) -> CommitStatus { - let mut stat = CommitStatus { - api, - repo, - sha, - context, - description, - url: String::new(), - }; - - stat.set_url(url); - - stat - } - - pub fn set_url(&mut self, url: Option) { - self.url = url.unwrap_or_default(); - } - - pub async fn set_with_description( - &mut self, - description: &str, - state: CheckRunState, - ) -> Result<(), CommitStatusError> { - self.set_description(description.to_owned()); - self.set(state).await - } - - pub fn set_description(&mut self, description: String) { - self.description = description; - } - - pub async fn set(&self, state: CheckRunState) -> Result<(), CommitStatusError> { - let desc = if self.description.len() >= 140 { - warn!( - "description is over 140 char; truncating: {:?}", - &self.description - ); - self.description.chars().take(140).collect() - } else { - self.description.clone() - }; - - self.api - .create_commit_statuses( - &self.repo, - self.sha.clone(), - state, - self.context.clone(), - desc, - self.url.clone(), - ) - .await - } -} - -#[derive(Debug)] -pub enum CommitStatusError { - ExpiredCreds(()), - MissingSha(()), - Error(()), -} diff --git a/ofborg/src/vcs/generic/mod.rs b/ofborg/src/vcs/generic/mod.rs index 1b3510a..bd450e5 100644 --- a/ofborg/src/vcs/generic/mod.rs +++ b/ofborg/src/vcs/generic/mod.rs @@ -1,8 +1,10 @@ pub mod api; +pub mod change_status; pub mod checks; pub mod commit_status; pub mod http; pub use api::*; +pub use change_status::*; pub use checks::*; pub use commit_status::*;