Skip to content

feat: added check for invalid username and if it is invalid find related names #200

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Gmin2
Copy link

@Gmin2 Gmin2 commented Feb 21, 2025

Fixes #191

Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! It looks pretty good already. Left some comments.

@@ -1,6 +1,7 @@
use anyhow::Context;
use octocrab::models::{App, Repository};
use octocrab::{Error, Octocrab};
use strsim::levenshtein;
Copy link
Contributor

Choose a reason for hiding this comment

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

The strtim dependency is missing.

Copy link
Author

Choose a reason for hiding this comment

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

oh shit, forgotten to push it, done now !

async fn find_similar_usernames(&self, username: &str) -> anyhow::Result<Vec<String>> {
const MAX_DISTANCE: usize = 2;

let search_results = self
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't use the GitHub search API for this, that could be slow, and unknown GH users are not relevant anyway. Instead, we should try to search for known users in the team database. It will also be easier to test it like this.

But we can also skip the "find similar" functionality in this PR, if you want.

Copy link
Author

@Gmin2 Gmin2 Feb 24, 2025

Choose a reason for hiding this comment

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

I wouldn't use the GitHub search API for this, that could be slow, and unknown GH users are not relevant anyway. Instead, we should try to search for known users in the team database. It will also be easier to test it like this.

so we should use this right ?

pub(crate) async fn load_permissions(

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the reviewer list from the team API.

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.

Check that reviewers actually exist
2 participants