Add an option to only announce PRs from specific users

[aracho1]

What does this change?

Co-authored-by: @lindseydew

We would like to introduce an option to make the PR announcer only announce PRs from specific users e.g. scala steward.

How to test

We have tested this on our MSS dev chat, which worked as expected.

How can we measure success?

We can now make the PR announcer alert PRs from a select list of users.

Have we considered potential risks?

We have set this new flag to be optional so existing announcers should not be affected.

[AshCorr]

I think announced_users will default to an empty vector? Would this mean that the default would be to ignore everyone?

Wondering if we need to make announced_users a Optional<Vec<&str>> instead and check if it exists here?

[aracho1]

That makes sense, thank you for the suggestion!

[AshCorr]

Alternatively check if the Vec is empty and skip this check if it is, I prefer the explicit Optional however.

[marsavar]

I agree with Ash here, with a preference for the option type.

Also, since you know user id's are of type i32, you could parse them here already so you avoid the awkward conversion &pull_request.user().id().to_string().as_str().

I.e.
You could do something like

    let announced_users: Option<Vec<i32>> = env::var("GITHUB_ANNOUNCED_USERS")
        .ok()
        .map(|s| s.split(',').map(|id| id.parse().unwrap()).collect());

And then change the signature of scan_repository so that it accepts announced_users: Option<&[i32]>

so the final check would look something like

        if let Some(announced_users) = announced_users {
            if !announced_users.contains(pull_request.user().id()) {
                info!("Users to announce: {:?}", announced_users);
                info!(
                "Ignoring PR {}({}) as it was raised by a user not included in the announced users list {}({})",
                pull_request.id(),
                pull_request.title(),
                pull_request.user().id(),
                pull_request.user().login()
            );
                continue;
            }
        }

[AshCorr]

Not quite sure how Github handles this if inputs.github-announced-users isn't defined, it might interpet it as an empty string:

GITHUB_ANNOUNCED_USERS: ""

In which case our code to check if the environment variable is present or not won't correctly detect that the GITHUB_ANNOUNCED_USERS is undefined. Sorry to suggest Option but now I'm thinking just checking the length of our announced users array might be better than checking if the variable is present or not!

[aracho1]

That's a good spot, thanks @AshCorr. How about we do something like this to handle the empty string?

let announced_users: Option<Vec<i32>> = env::var("GITHUB_ANNOUNCED_USERS")
        .ok()
        .and_then(|s| {
            if s.is_empty() {
                None
            } else {
                Some(s.split(',').map(|id| id.parse().unwrap()).collect())
            }
        });

[AshCorr]

Aha! Yea that would work!

[aracho1]

Just pushed the change, happy to +1 @AshCorr ?

[AshCorr]

Fantastic 👍