ia-get
ia-get copied to clipboard
ia-get feedback and request to contribute PRs
Hi Martin,
We met in Riga and sat in the hotel bar with Popey and others until the early morning talking about Ubuntu, Ubuntu Hideout, Bottom, Danger Brothers, and the other exploits of Rik Mayall and Ade Edmondson. It was lovely to meet you both and share a few laughs. (This night )
Feedback
Regarding: Linux Matters Episode 20
The application itself looks good! I appreciate that your are conducting an experiment on the merits of AI Driven Development :TM: but since you mentioned contributions on Episode 20 of Linux Matters I thought I would offer these thoughts. They might be of interest to you to see where an external programmer can spot contextual/ ecosystem deficits in the generated answers. I would like to create a PR for as many of the following things as you are comfortable allowing contributors address.
Documentation
I would like to contribute comment-documentation for the functions, structures and Rust module which will be converted into proper Rust documentation when cargo doc
is executed.
is_url_accessible
error management
is_url_accessible
returns a Result, but no error is ever propagated from the function - only Ok(true)
or Ok(false)
. This means that error handling here and here never reaches the Err
branches.
This can be corrected as follows:
async fn is_url_accessible(url: &str) -> Result<bool, Box<dyn Error>> {
let client = reqwest::Client::new();
let response = client.get(url).send().await?; // ? will propagate errors
Ok(response.status().is_success()) // If your goal is to return `true` and `false` as Ok, then this will work.
}
.is_success()
checks for HTTP response code 200-299 so I suspect you actually would want to handle the false
case as an error as well e.g:
async fn is_url_accessible(url: &str) -> Result<bool, Box<dyn Error>> {
let client = reqwest::Client::new();
let response = client.get(url).send().await?;
response.error_for_status()?; // Propagate an error if HTTP response code is 400 - 599
Ok(true) // Only return Ok if no transport or HTTP error is encountered.
}
- This will correct some currently hidden errors
- Your existing error handling logic at the call-sites will be compatible with this although further updates can be made to improve the function return semantics:
async fn is_url_accessible(url: &str) -> Result<(), Box<dyn Error>> {
let client = reqwest::Client::new();
let response = client.get(url).send().await?;
response.error_for_status()?;
Ok(())
}
The call-sites then manage the errors by matching Ok(())
or Err(e)
e.g.
match is_url_accessible(details_url).await {
Ok(()) => println!("╰╼ Archive.org URL online: 🟢"), // Ok(true)/ Ok(false)/Err(e) semantics replaced by Ok(())/Err(e)
Err(e) => {
println!("├╼ Archive.org URL online: 🔴");
panic! ("╰╼ Exiting due to error: {}", e);
}
}
and
match is_url_accessible(&xml_url).await {
Ok(()) => println!("├╼ Archive.org XML online: 🟢"),
Err(e) => {
println!("├╼ Archive.org XML online: 🔴");
panic! ("╰╼ Exiting due to error: {}", e);
}
}
is_url_accessible
request type
Instead of performing a GET
request, it might be better internet citizenry to perform a HEAD
request since no response body is required.
let response = client.head(url).send().await?;
Test cases for PATTERN
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn check_valid_pattern() {
let regex = Regex::new(&PATTERN).expect("Create regex");
assert!(regex.is_match("https://archive.org/details/Valid-Pattern"))
}
#[test]
fn check_invalid_pattern() {
let regex = Regex::new(&PATTERN).expect("Create regex");
assert!(!regex.is_match("https://archive.org/details/Invalid-Pattern-*"))
}
}
PATTERN syntax
The escape sequences \/
are unnecessary and can be replaced with /
only:
static PATTERN: &str = r"^https://archive\.org/details/[a-zA-Z0-9_-]+$";
Valid sequences can be seen here.
The aforementioned test cases demonstrate this works.
Request user-agent
It might be considered good internet citizenry to set the application user-agent to IA-Get
for requests.
Packaging
I would like to help create a snapcraft.yaml
. I have an existing file which I should be able to recraft.
I hope these thoughts are received in the good spirit they are intended and if you decide to incorporate any of these you would permit me to submit the commits for them.
Best regards
Daniel