About move forward the command /LGTM for reviewing a PR

Hi, guys.

We are going to promote commands /LGTM for reviewing and approving a PR and all repos should support this behavior universally and consistently.

With this command, we can make our reviewing process more clearly than before. And according to this command, we can make sure the reviewers have already tested and approved this PR. It can help increase the code quality.

Here is an original comment from Lee.

1 Like

For my part, I’m generally in favor of tools like this and think that such tools should be considered for use across all five of the GitHub organizations that we maintain. I do have a couple of concerns with the specific approach proposed, however.

Tools like this not only aid maintainers, but can potentially enable contributors beyond what their normal permissions would otherwise be, which is great. However, care and consideration have to be given as to who can invoke which slash commands.

Example slash command on a PR: /assign @someone @someone-else to assign to one or more people with the behavior of this command being additive and idempotent to any existing assignments or /assign to have the issue self-assigned.

Concerns to addresss:

  1. Use of existing tooling - Ideally, research should be done to find an existing bot or existing set of commands and actions that other projects are using as opposed to crafting something from scratch.
    1. Integration with Slack?
    2. Use in issues and pull requests?
  2. Permissions to prevent abuse - Any contributor can leave a comment and potentially invoke one of these slash commands inappropriately. Unless research from #1 doesn’t include this consideration, then I propose use GitHub team membership as a method to structure permissions around which individuals can invoke with commands.
    1. If contributors that are not part of the GitHub org are allow to use /assign to self-assign, the potential for abuse (intentional or accidental is high). Currently, we see high rates of would be contributors clobbering each other to be assigned to open help-wanted issues. Or asking to be assigned to very complex issues that they are clearly not prepared to undertake, but continue to ask to be assigned anyway. If they are empowered to do so, onto their own, we increase the likelihood for heartache of other contributors that might get their toes stepped on by a different contributor self-assigning.
  3. Documentation - all commands, permissions, and behaviors are documented in docs.meshery.io for ease of reference by all contributors.
  4. Discovery - Issue and PR templates are updated to include a link to command documentation, so that contributors can become aware of the availability of such commands. Even when links are offered in these templates, we have seen low rates of contributors actually reading them, so it’s best to have low expectations of contributors actually using them. This is to say that no process should be built around the expiation that contributors will be aware of the availability of these commands.
2 Likes