Skip to content

Parse --repo owner/name input in analyze command#8

Merged
marcoieni merged 4 commits into
rust-lang:mainfrom
Sandijigs:analyze-cache-repo
Jun 19, 2026
Merged

Parse --repo owner/name input in analyze command#8
marcoieni merged 4 commits into
rust-lang:mainfrom
Sandijigs:analyze-cache-repo

Conversation

@Sandijigs

Copy link
Copy Markdown
Contributor

This is the first small piece of the analyze command .When --repo is passed to analyze, the value is split on / and rejected unless it has exactly two non-empty parts. For now it just prints the parsed owner and repo as a sanity check,
Also added anyhow so main can return Result<()> and use bail! for the error message.

Comment thread src/main.rs Outdated
}

fn main() {
fn main() -> Result<()> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like importing result from anyhow, let's specify anyhow::Result every time 😅

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Dropped Result from the import and spelled out anyhow::Result in the signature.

Comment thread src/main.rs Outdated
}
let (org, repo) = (parts[0], parts[1]);
println!("parsed org={org} repo={repo}");
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the main function should be small, let's have an analyze function in an analyze.rs file under a command directory.
Then have another function that parses a string and returns a struct with org and repo
Then you can add a test that tests that this function works

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have Moved the analyze logic into src/command/analyze.rs, added a ParsedRepo struct with parse_repo, and added tests for the happy path and four failure cases.

Comment thread src/command/analyze.rs
use anyhow::bail;

#[derive(Debug, PartialEq)]
pub struct ParsedRepo {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Repo is enough but can be changed in another PR

@marcoieni marcoieni merged commit cf80c88 into rust-lang:main Jun 19, 2026
4 checks passed
@marcoieni

Copy link
Copy Markdown
Member

please squash your commits next time if the PR is ready to be merged 🙏
This time I merged the PR because it's not a big deal

@Sandijigs

Copy link
Copy Markdown
Contributor Author

please squash your commits next time if the PR is ready to be merged 🙏 This time I merged the PR because it's not a big deal

Noted . I will definately put that in mind next time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants