Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: apply clippy::pedantic lints & improve CI #62

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,40 @@ on:
jobs:

simple-build:
env:
CARGO_TERM_COLOR: always
TERM: xterm-256color
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-latest]
# macos-13 for x86_64-darwin
os: [ubuntu-latest, macos-13, macos-latest]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- uses: cachix/install-nix-action@v30
with:
nix_path: nixpkgs=channel:nixos-unstable
- uses: DeterminateSystems/magic-nix-cache-action@v8
- run: nix-build
- run: nix-shell --command 'echo $version' # set in package.nix

flakes:
env:
CARGO_TERM_COLOR: always
TERM: xterm-256color
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-latest]
os: [ubuntu-latest, macos-13, macos-latest]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- uses: cachix/install-nix-action@v30
with:
extra_nix_config: |
access-tokens = github.com=${{ secrets.GITHUB_TOKEN }}
- uses: DeterminateSystems/magic-nix-cache-action@v8
- run: nix build
- run: nix build --print-build-logs
- run: nix flake check
- run: nix develop -c cargo clippy
- run: nix develop -c cargo fmt --check --all
- run: nix develop -c cargo test --all-features -- --color=always --ignored # run only ignored tests
15 changes: 15 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
[package]
name = "hydra-check"
version = "2.0.1"
description = "Check hydra for the build status of a package"
authors = ["Felix Richter <[email protected]>", "Bryan Lai <[email protected]>"]
edition = "2021"
license = "MIT"
repository = "https://github.com/nix-community/hydra-check"
keywords = ["cli"]
categories = ["command-line-utilities"]

[dependencies]
anyhow = "1.0.89"
Expand All @@ -26,3 +31,13 @@ insta = "1.41.1"
[profile.dev.package]
insta.opt-level = 3
similar.opt-level = 3

[lints.clippy]
pedantic = { level = "warn", priority = -1 }
cargo = { level = "warn", priority = -1 }
manual_string_new = "allow"
match_bool = "allow"
single_match = "allow"
single_match_else = "allow"
missing_errors_doc = "allow"
multiple_crate_versions = "allow"
20 changes: 12 additions & 8 deletions src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ pub(crate) enum Queries {

#[derive(Parser, Debug, Default)]
#[command(author, version, verbatim_doc_comment)]
#[allow(rustdoc::bare_urls)]
#[allow(
rustdoc::bare_urls,
clippy::doc_markdown,
clippy::struct_excessive_bools
)]
#[deny(missing_docs)]
///
/// Check hydra.nixos.org for build status of packages
Expand Down Expand Up @@ -244,16 +248,16 @@ impl HydraCheckCli {
);
None
} else {
Some(self.guess_package_name(&package))
Some(self.guess_package_name(package))
}
})
.collect()
}

fn guess_evals(&self) -> Vec<Evaluation> {
let mut evals = Vec::new();
for spec in self.queries.iter() {
evals.push(Evaluation::guess_from_spec(spec))
for spec in &self.queries {
evals.push(Evaluation::guess_from_spec(spec));
}
evals
}
Expand Down Expand Up @@ -338,8 +342,8 @@ impl ResolvedArgs {
self.fetch_and_print_jobset(self.short)?;
Ok(true)
}
Queries::Packages(packages) => self.fetch_and_print_packages(&packages),
Queries::Evals(evals) => self.fetch_and_print_evaluations(&evals),
Queries::Packages(packages) => self.fetch_and_print_packages(packages),
Queries::Evals(evals) => self.fetch_and_print_evaluations(evals),
}
}
}
Expand All @@ -354,7 +358,7 @@ fn guess_jobset() {
for (channel, jobset) in aliases {
eprintln!("{channel} => {jobset}");
let args = HydraCheckCli::parse_from(["hydra-check", "--channel", channel]).guess_jobset();
debug_assert_eq!(args.jobset, Some(jobset.into()))
debug_assert_eq!(args.jobset, Some(jobset.into()));
}
}

Expand All @@ -375,5 +379,5 @@ fn guess_darwin() {
#[ignore = "require internet connection"]
fn guess_stable() {
let args = HydraCheckCli::parse_from(["hydra-check", "--channel", "stable"]).guess_jobset();
eprintln!("{:?}", args.jobset)
eprintln!("{:?}", args.jobset);
}
2 changes: 1 addition & 1 deletion src/fetch_stable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,5 @@ fn fetch_stable() {
println!("latest stable version: {ver}");
debug_assert!(regex::Regex::new(r"^[0-9]+\.[0-9]+")
.unwrap()
.is_match(&ver))
.is_match(&ver));
}
3 changes: 2 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//!
//! </div>
//!
#![allow(clippy::doc_markdown)]
#![doc = include_str!("../README.md")]

mod args;
Expand Down Expand Up @@ -58,7 +59,7 @@ trait FetchHydraReport: Clone {
/// Checks if the fetched [Html] contains a `tbody` tag (table body).
/// If not, returns the alert text. If yes, returns the found element.
fn find_tbody<'a>(&self, doc: &'a Html, selector: &str) -> Result<ElementRef<'a>, Self> {
let selectors = format!("{} tbody", selector);
let selectors = format!("{selector} tbody");
match doc.find(selectors.trim()) {
Err(_) => {
// either the package was not evaluated (due to being e.g. unfree)
Expand Down
43 changes: 22 additions & 21 deletions src/queries/evals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ fn format_eval_input() {
value: https://github.com/nixos/nixpkgs.git
revision: 1e9e641a3fc1b22fbdb823a99d8ff96692cc4fba
store_path: /nix/store/ln479gq56q3kyzyl0mm00xglpmfpzqx4-source
"#)
"#);
}

#[skip_serializing_none]
Expand Down Expand Up @@ -130,12 +130,14 @@ fn format_input_changes() {
changes: 8c4dc69b9732 to 1e9e641a3fc1
url: https://hydra.nixos.org/api/scmdiff?blah/blah/blah
revs: 8c4dc69b9732f6bbe826b5fbb32184987520ff26 -> 1e9e641a3fc1b22fbdb823a99d8ff96692cc4fba
"#)
"#);
}

impl EvalInputChanges {
#[allow(clippy::similar_names)]
fn from_html(doc: &Html) -> anyhow::Result<Vec<Self>> {
let tables = doc.find_all("div#tabs-inputs table");
#[allow(clippy::redundant_closure_for_method_calls)]
let err = || {
anyhow!(
"could not parse the table of changed inputs in {:?}",
Expand Down Expand Up @@ -175,7 +177,7 @@ impl EvalInputChanges {
.find("a")
.ok()
.and_then(|x| x.attr("href"))
.map(|x| x.to_string());
.map(str::to_string);

let revs = if let Some(url) = &url {
// note that the returned url is not deterministic:
Expand All @@ -196,7 +198,9 @@ impl EvalInputChanges {
None
};

let short_revs = if !description.is_empty() {
let short_revs = if description.is_empty() {
None
} else {
match Regex::new("^([0-9a-z]+) to ([0-9a-z]+)$")
.unwrap()
.captures(&description)
Expand All @@ -207,8 +211,6 @@ impl EvalInputChanges {
}
_ => None,
}
} else {
None
};

input_changes.push(EvalInputChanges {
Expand Down Expand Up @@ -281,7 +283,7 @@ impl<'a> From<&'a Evaluation> for EvalReport<'a> {
}
}

impl<'a> EvalReport<'a> {
impl EvalReport<'_> {
fn parse_build_stats(&self, doc: &Html, selector: &str) -> anyhow::Result<Vec<BuildStatus>> {
let err = || {
anyhow!(
Expand All @@ -290,7 +292,7 @@ impl<'a> EvalReport<'a> {
doc.html()
)
};
let tbody = match self.find_tbody(&doc, selector) {
let tbody = match self.find_tbody(doc, selector) {
Err(stat) => bail!("{:?}", stat.inputs.first().ok_or_else(err)?.value),
Ok(tbody) => tbody,
};
Expand All @@ -312,12 +314,13 @@ impl<'a> EvalReport<'a> {
.map(|x| {
let text: String = x.text().collect();
match text.trim() {
x if x.is_empty() => None,
"" => None,
x => Some(x.to_string()),
}
})
.collect();
let [name, input_type, value, revision, store_path] = columns.as_slice() else {
#[allow(clippy::redundant_else)]
if let Ok(true) = is_skipable_row(row) {
continue;
} else {
Expand Down Expand Up @@ -379,13 +382,10 @@ impl<'a> EvalReport<'a> {
}

impl ResolvedArgs {
pub(crate) fn fetch_and_print_evaluations(
&self,
evals: &Vec<Evaluation>,
) -> anyhow::Result<bool> {
pub(crate) fn fetch_and_print_evaluations(&self, evals: &[Evaluation]) -> anyhow::Result<bool> {
let mut indexmap = IndexMap::new();
let evals = match evals.iter().any(|eval| eval.id == 0) {
false => evals.clone(),
let evals: Vec<_> = match evals.iter().any(|eval| eval.id == 0) {
false => evals.to_owned(),
true => {
info!(
"querying the latest evaluation of --jobset '{}'",
Expand All @@ -397,9 +397,9 @@ impl ResolvedArgs {
self.jobset
)
};
eprintln!("");
eprintln!();
let id = self.fetch_and_print_jobset(false)?.ok_or_else(err)?;
println!("");
println!();
evals
.iter()
.map(|eval| match &eval.id {
Expand All @@ -418,7 +418,7 @@ impl ResolvedArgs {
if !self.json {
// print title first, then fetch
if idx > 0 && !self.short {
println!(""); // vertical whitespace
println!(); // vertical whitespace
}
println!(
"Evaluation {}{} {}",
Expand All @@ -436,11 +436,11 @@ impl ResolvedArgs {
continue;
}
for entry in &stat.inputs {
println!(""); // vertical separation
println!(); // vertical separation
println!("{entry}");
}
for entry in &stat.changes {
println!(""); // vertical separation
println!(); // vertical separation
println!("{entry}");
}
if self.short {
Expand All @@ -456,8 +456,9 @@ impl ResolvedArgs {
(&stat.still_succeed, "Still Succeeding:".bold()),
(&stat.unfinished, "Queued Jobs:".bold()),
] {
#[allow(clippy::uninlined_format_args)]
if !build_stats.is_empty() {
println!("");
println!();
println!("{}", prompt);
println!("{}", stat.format_table(false, build_stats));
}
Expand Down
11 changes: 6 additions & 5 deletions src/queries/jobset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ impl ShowHydraStatus for EvalStatus {
let delta = format!(
"Δ {}",
match self.delta.clone().unwrap_or("~".into()).trim() {
x if x.starts_with("+") => x.green(),
x if x.starts_with("-") => x.red(),
x if x.starts_with('+') => x.green(),
x if x.starts_with('-') => x.red(),
x => x.into(),
}
)
Expand Down Expand Up @@ -118,7 +118,7 @@ impl<'a> From<&'a ResolvedArgs> for JobsetReport<'a> {
}
}

impl<'a> JobsetReport<'a> {
impl JobsetReport<'_> {
fn fetch_and_read(self) -> anyhow::Result<Self> {
let doc = self.fetch_document()?;
let tbody = match self.find_tbody(&doc, "") {
Expand All @@ -131,6 +131,7 @@ impl<'a> JobsetReport<'a> {
let [eval_id, timestamp, input_changes, succeeded, failed, queued, delta] =
columns.as_slice()
else {
#[allow(clippy::redundant_else)]
if is_skipable_row(row)? {
continue;
} else {
Expand Down Expand Up @@ -161,7 +162,7 @@ impl<'a> JobsetReport<'a> {
let input_changes = {
let text: String = input_changes.text().collect();
let text = text.replace(&status, "");
let texts: Vec<_> = text.trim().split_whitespace().collect();
let texts: Vec<_> = text.split_whitespace().collect();
texts.join(" ")
};

Expand Down Expand Up @@ -201,7 +202,7 @@ impl<'a> JobsetReport<'a> {
failed: Some(failed?),
queued: Some(queued?),
delta,
})
});
}
Ok(Self { evals, ..self })
}
Expand Down
6 changes: 3 additions & 3 deletions src/queries/packages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl<'a> PackageReport<'a> {
}

impl ResolvedArgs {
pub(crate) fn fetch_and_print_packages(&self, packages: &Vec<String>) -> anyhow::Result<bool> {
pub(crate) fn fetch_and_print_packages(&self, packages: &[String]) -> anyhow::Result<bool> {
let mut status = true;
let mut indexmap = IndexMap::new();
for (idx, package) in packages.iter().enumerate() {
Expand All @@ -78,7 +78,7 @@ impl ResolvedArgs {
if !self.json {
// print title first, then fetch
if idx > 0 && !self.short {
println!(""); // vertical whitespace
println!(); // vertical whitespace
}
println!(
"Build Status for {} on jobset {}",
Expand Down Expand Up @@ -110,7 +110,7 @@ impl ResolvedArgs {
}
println!("{}", stat.format_table(self.short, &stat.builds));
if !success && self.short {
warn!("latest build failed, check out: {}", url_dimmed)
warn!("latest build failed, check out: {url_dimmed}");
}
}
if self.json {
Expand Down
3 changes: 2 additions & 1 deletion src/soup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ use anyhow::anyhow;
use scraper::{selectable::Selectable, ElementRef, Html, Selector};
use std::fmt::Debug;

#[allow(clippy::module_name_repetitions)]
/// A simple wrapper trait that provides the `find` and `find_all` methods
/// to [`scraper`]'s [`Selectable`] elements, inspired by the interface of
/// Python's BeautifulSoup.
/// Python's `BeautifulSoup`.
pub trait SoupFind<'a> {
/// Finds all child elements matching the CSS selectors
/// and collect them into a [`Vec`].
Expand Down
Loading
Loading