Skip to content

Commit

Permalink
fix: review plugin default policy and return type
Browse files Browse the repository at this point in the history
  • Loading branch information
j-lanson authored and mchernicoff committed Oct 30, 2024
1 parent a3bc81e commit 186ca5d
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 31 deletions.
3 changes: 1 addition & 2 deletions hipcheck/src/plugin/retrieval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,12 @@ use xz2::read::XzDecoder;
use super::get_current_arch;

/// The plugins currently are not delegated via the `plugin` system and are still part of `hipcheck` core
pub const MITRE_LEGACY_PLUGINS: [&str; 7] = [
pub const MITRE_LEGACY_PLUGINS: [&str; 6] = [
"activity",
"entropy",
"affiliation",
"binary",
"churn",
"review",
"typo",
];

Expand Down
10 changes: 5 additions & 5 deletions plugins/review/plugin.kdl
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ name "review"
version "0.1.0"
license "Apache-2.0"
entrypoint {
on arch="aarch64-apple-darwin" "./hc-mitre-review"
on arch="x86_64-apple-darwin" "./hc-mitre-review"
on arch="x86_64-unknown-linux-gnu" "./hc-mitre-review"
on arch="x86_64-pc-windows-msvc" "./hc-mitre-review"
on arch="aarch64-apple-darwin" "./target/debug/review_sdk"
on arch="x86_64-apple-darwin" "./target/debug/review_sdk"
on arch="x86_64-unknown-linux-gnu" "./target/debug/review_sdk"
on arch="x86_64-pc-windows-msvc" "./target/debug/review_sdk"
}

dependencies {
plugin "mitre/github_api" version="0.1.0" manifest="./plugins/github_api/plugin.kdl"
}
}
36 changes: 12 additions & 24 deletions plugins/review/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use std::{result::Result as StdResult, sync::OnceLock};

#[derive(Deserialize)]
struct Config {
#[serde(rename = "percent-threshold")]
percent_threshold: Option<f64>,
}

Expand All @@ -27,9 +28,9 @@ pub struct PullReview {
pub has_review: bool,
}

/// Returns the percentage of commits to the repo that were merged without review
#[query]
async fn review(engine: &mut PluginEngine, value: Target) -> Result<f64> {
/// Returns whether each commit in a repo was merged with a review
#[query(default)]
async fn review(engine: &mut PluginEngine, value: Target) -> Result<Vec<bool>> {
log::debug!("running review metric");

// Confirm that the target is a GitHub repo
Expand Down Expand Up @@ -62,28 +63,11 @@ async fn review(engine: &mut PluginEngine, value: Target) -> Result<f64> {
// Create a Vec big enough to hold every single pull request
let mut pull_reviews = Vec::with_capacity(pull_requests.len());

// Create a list of pull requesets, with a boolean indicating if they have at least one review or not
for pull_request in pull_requests {
let has_review = pull_request.reviews > 0;
pull_reviews.push(PullReview {
pull_request,
has_review,
});
}

// Calculate the percentage of unreviewed pull requests out of the total
// If there are no pull requests, return 0.0 to avoid a divide-by-zero error
let num_flagged = pull_reviews.iter().filter(|p| !p.has_review).count() as u64;
let percent_flagged = match (num_flagged, pull_reviews.len()) {
(flagged, total) if flagged != 0 && total != 0 => {
num_flagged as f64 / pull_reviews.len() as f64
}
_ => 0.0,
};
pull_reviews.extend(pull_requests.into_iter().map(|pr| pr.reviews > 0));

log::info!("completed review query");

Ok(percent_flagged)
Ok(pull_reviews)
}

#[derive(Clone, Debug)]
Expand All @@ -95,6 +79,7 @@ impl Plugin for ReviewPlugin {
const NAME: &'static str = "review";

fn set_config(&self, config: Value) -> StdResult<(), ConfigError> {
println!("config: {config:?}");
let conf =
serde_json::from_value::<Config>(config).map_err(|e| ConfigError::Unspecified {
message: e.to_string(),
Expand All @@ -110,7 +95,10 @@ impl Plugin for ReviewPlugin {
return Err(Error::UnspecifiedQueryState);
};
match conf.percent_threshold {
Some(threshold) => Ok(format!("lte $ {}", threshold)),
Some(threshold) => Ok(format!(
"(lte (divz (count (filter (eq #f) $)) (count $)) {})",
threshold
)),
None => Ok("".to_owned()),
}
}
Expand Down Expand Up @@ -186,7 +174,7 @@ mod test {
let mut engine = PluginEngine::mock(mock_responses().unwrap());
let result = review(&mut engine, target).await.unwrap();

let expected = 0.25;
let expected = vec![true, true, false, true];

assert_eq!(result, expected);
}
Expand Down

0 comments on commit 186ca5d

Please sign in to comment.