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

Model changes by contractors are stalled. #15020

Open
jvasilje opened this issue Nov 13, 2024 · 4 comments
Open

Model changes by contractors are stalled. #15020

jvasilje opened this issue Nov 13, 2024 · 4 comments
Assignees
Labels
bug Something isn't working P0

Comments

@jvasilje
Copy link
Collaborator

jvasilje commented Nov 13, 2024

MCW can't merge a bunch of PRs. The list of reviewers is large, and it even includes folks from the Moreh team, which is extremely inefficient.

As first step, I'd like the list of reviewers to be reduced - something is very wrong with the current flow. It's either our folder structure, of code reviewers structure.

Some data:

List of models that are blocked (look at column G)
https://docs.google.com/spreadsheets/d/1Dx0QixtRB4_JZ6psOX6q4LNaW40-p8zJ_XTWK2Ootdg/edit?gid=1795313225#gid=1795313225

An example issue:
https://github.com/tenstorrent/tt-metal/pull/13475/files

@jvasilje jvasilje added P0 bug Something isn't working labels Nov 13, 2024
@bkeith-TT
Copy link

Actions:

  • Determine current 'average' time to code review
  • Determine best practice time to code review
  • Brainstorm solutions
  • Implement solutions

@jvasilje
Copy link
Collaborator Author

fyi @zzigler-tt @boris-drazic

Let's work on this with some expediency. To help, here is a list of Pending PRs from MCW
https://docs.google.com/spreadsheets/d/1Dx0QixtRB4_JZ6psOX6q4LNaW40-p8zJ_XTWK2Ootdg/edit?gid=470056989#gid=470056989

When I look at these PRs, I see the following dates:
Oct 3
Jun 11
Sep 13
Oct 4
May 9
...

Here's an example PR with 19 reviewers.
#8339

I'd like us to review the folders and code owners as a first pass.
First round of issues, as I see it:

  • too many reviewers
  • Moreh is triggered to review model demo changes. This makes zero sense to me.

@zzigler-tt
Copy link

Also, are the approvers receiving notifications thats there are PR's pending review? I am trying to understand how we can have these items sitting for such a long time.

@jvasilje
Copy link
Collaborator Author

@zzigler-tt even if you get pinged to review/approve a PR, and you see 19 REVIEWERS listed there, you are likely to do nothing and assume that someone else will handle it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P0
Projects
None yet
Development

No branches or pull requests

3 participants