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

Support usernames blacklist for a repository #18

Open
garno opened this issue Jun 20, 2019 · 2 comments
Open

Support usernames blacklist for a repository #18

garno opened this issue Jun 20, 2019 · 2 comments
Labels
enhancement New feature or request rfc Request for comments on new features

Comments

@garno
Copy link
Member

garno commented Jun 20, 2019

Sometimes, someone might feel like he has lost track of a projet for various reasons. If that is the case, he might not be the appropriate person to review a Pull Request and he’s taking the spot of a more relevant reviewer.

To prevent this, we could allow a per-repository configuration with a blacklist.

{
  "repositories": {
    "mirego/dispatch": {
      "blacklist": [
        {
          "username": "garno"
        }
      ]
    }
  }
}

By keeping the same structure as the global blacklist, we can simply merge both of them before making the final reviewers selection.

This is the first time we’re adding repository configuration outside the query params. Thoughts? It could be use for various other improvements.

✌️

@garno garno added enhancement New feature or request rfc Request for comments on new features labels Jun 20, 2019
@remi
Copy link
Member

remi commented Jun 21, 2019

Since the organization never changes, it’s not possible for Dispatch to handle a Webhook URL from another organization. Therefore I think we could drop the mirego/ from the repository key.

However, I’m not sure I’m on board with the structure you’re suggesting here, since it opens up an opportunity to store repository settings in the JSON file rather than trough URL query parameters. We want to avoid configuration.json becoming a database file (eg. by storing repository stacks and other settings in it).

I think we could extend our already-existing blacklist format to support specific repositories:

{
  "blacklist": [
    {
      "username": "foo"
    },
    {
      "username": "bar",
      "repositories": ["repository-bar-no-longer-wants-to-be-requested-on"]
    }
  ]
}

The behavior for the user foo doesn’t change, he’ll still be blacklisted for all repositories. However, bar will only be considered blacklisted for Webhook requests related to this specific repository.

What do you think?

@garno
Copy link
Member Author

garno commented Jun 21, 2019

We want to avoid configuration.json becoming a database file (eg. by storing repository stacks and other settings in it).

I feel like it is already the case though. I think that using query parameters will eventually add an unnecessary complexity as we add more and more preferences for a repository.

That being said, I do not dislike the structure you are proposing. @thermech will also approve :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request rfc Request for comments on new features
Projects
None yet
Development

No branches or pull requests

2 participants