Code Review Process Update

@tomasreimers Any updates on this?

So, LLVM switched to GH PRs. What is the way to test-drive Graphite?

I could demo it to you over zoom if that works? Feel free to shoot me an email: tomas at graphite.dev :slight_smile:

Very relevant, and shamelessly asking for support: we just launched–for the first time–today on Product Hunt. If you have a moment, and an account, your support would mean the world to us. Thank you!

On reviewable.io.

I tried to use it for a review, but I could not post the results as the llvm org does not prove the necessary authorization scope for the app. On pushing the publish button, I get an error like this:
Failed to publish [...], the llvm organization has enabled OAuth App access restrictions, [...] visit https://docs.github.com/articles/restricting-access-to-your-organization-s-data/

On reviewable, I can see these two scopes granted: public_repo, user:email

However, it also appears to need the “repo” scope, to post your review.
Here are the docs of the “repo” scope:

Grants full access to public and private repositories including read and write access to code, commit statuses, repository invitations, collaborators, deployment statuses, and repository webhooks. Note: In addition to repository related resources, the repo scope also grants access to manage organization-owned resources including projects, invitations, team memberships and webhooks. This scope also grants the ability to manage projects owned by users.

By reading the scope, it appears to me that any other review assistant tool/website would need this authorization.

Do you know someone how could grant the “repo” scope for the llvm org?
Or is this deliberately not granted?

This question has been asked several times since Sept both in public and in private and we’ve never gotten an answer. As best I can tell, nobody feels comfortable granting the permissions to the tool, but it’s also entirely unclear how to get the administrators to engage on the topic.

In the November Board meeting there was a discussion about creating a policy for enabling third-party apps. I think we are still waiting on this.

1 Like

Thank you for the update! Unfortunately, I’m still concerned by this situation.

We were asking about reviewable.io in 2020, long before the switch in Sept 2023 (Phabricator -> GitHub PRs? - #154 by jyknight) and it was already observed that we needed permissions (Phabricator -> GitHub PRs? - #157 by akorobeynikov), so I’m really concerned that this discussion is only getting started in Nov 2023 with no visible progress by the end of Jan 2024.

LLVM Foundation board members (@rnk @beanz, feel free to tag others, I don’t want to spam folks): how can we reduce the latency here given how significant the impacts are? It’s worrying that the LLVM Foundation is a gatekeeper and a bottleneck because the community can’t really start to evaluate whether those alternative solutions work for us until the Foundation allows us to. I’m not asking to rush decisions, but the foundation’s monthly board meeting schedule doesn’t necessarily align with the community’s needs and so there might be a slight disconnect in terms of priority regarding the need to improve the contributor experience after having made the switch to PRs.

7 Likes

Hey @AaronBallman & @tstellar,

I’m going to take two action items for the board meeting on Friday (1) to approve reviewable.io independent of the policy, and (2) to have the board review the policy draft that I’ve put together.

When we discussed this in the board meeting I did not believe that we were waiting on this policy to be in place before enabling reviewable.io. It was not the intention of the board to get in the way or slow this down. I apologize that we seem to have done just that.

The concern that we had was really centered around respecting the privacy of contributors. We want to make sure that contributors are notified of any third parties that the LLVM organization is sharing data with before the sharing occurs.

That said, I believe that GitHub largely does the right thing here. My understanding is that when a GitHub organization grants access it only gets organization information and individual users have to separately allow the application to have access to their user data.

Let’s make sure this is unblocked ASAP.

6 Likes

Thank you for the quick response (and for keeping our contributor’s privacy in mind, that’s sincerely appreciated)!

2 Likes

The LLVM Foundation board voted to allow enabling reviewable.io and graphite.dev on the LLVM organization with two caveats:

  1. It is our understanding that the applications are only requesting access to public repository data, and publicly available user information, and that if the application requires private user information individual users will be prompted for consent.
  2. Once a policy is in place for reviewing 3rd party GitHub applications we will retroactively review these two applications to ensure they adhere to community policies.

I’m working on a draft policy for enabling 3rd party applications. In our meeting minutes we’ve called this a “privacy policy”, but I believe the scope may extend beyond just privacy concerns (GitHub applications can do a lot of undesirable things with just public information). When I have a draft of the policy ready I will post an RFC on Discourse to gather community feedback.

In the interim, if additional GitHub applications need to be enabled to unblock community progress please contact the board (or any of the individual board members) and we’ll try to take quick action.

5 Likes

Both reviewable.io and graphite.dev were enabled for llvm-project/llvm repo. Please let me know if there are issues

7 Likes

Thank you @beanz and @akorobeynikov, I appreciate your help with this!

1 Like