-
Notifications
You must be signed in to change notification settings - Fork 588
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
Add Team Reviewers to Pull Request #1126
Comments
I'm not well-versed in graphql at all. It's rather easy to find all teams of the parent owner organization of a repository, but not all of those teams would have access to that repository, so we would have to get the list of repositories per team and then filter in-app? |
This is the one feature that would stop me needing to go into the web view at all. 🙏 |
We're starting endgame next week and the PR for this isn't yet ready to be merged (and even if it we get it merged tomorrow it won't have much selfhost time in the pre-release build). Moving to March, but it should be available in pre-release in a week or two! |
Moving this out, I hope for the last time. The reason I haven't merged it yet is that we're waiting to see if GitHub can provide a way to get team reviewers without adding the |
* Add Team Reviewers to Pull Request Fixes #1126 * Fix conflicts * Delay new scopes until teams are requested * Feedback
I love this functionality in theory, but in practice, it has made the "Reviewers" selector almost unusable on my machine. It takes several seconds to pull in the list items, sometimes not finishing before the selector goes away. How often does it try to grab collaborators and/or teams from the GitHub API? Can it be made to cache the downloaded data better? This behavior seems similar to the problems described in #4814. Could the fix for that issue help in this situation? |
@curtisgibby we shouldn't be pulling reviewers every time you open the selector. I think I see the problem that would be causing the refresh to happen for you every time though. |
Actually, I don't see what's going on. I'm seeing the cache get used properly. @curtisgibby can you share logs from "GitHub Pull Request" next time you see this? |
The first time that I created a PR (16:40 yesterday afternoon), it went quite quickly. Today when I tried to attach reviewers to another new PR, it went slowly.
|
The cached team reviewers are being used correctly in those logs. @curtisgibby can you confirm that these are your steps?
|
@alexr00 yep, that's my process. 🤷🏻 |
Ok, I'm making a change to further improve the reviewer caching and add more logging. @curtisgibby, the change will be in the pre-release build of the extension tomorrow. If you are able to try out the pre-release version of the extension once the changes are available tomorrow and do the following that would help me understand what's going on:
|
Thanks for your patience. I had to figure out how to install the pre-release version. I read these log entries as saying the first query took 38 seconds, and the second took 2.2 seconds. 😥
|
Thanks @curtisgibby! Looks like we're pretty quick to get the actual users, it's getting them ready to show that's slow. I suspect this slowness is coming from downloading all the avatars. I'm going to try not including the avatars if we have a large number of reviewers. |
@curtisgibby do you happen to know how many assignable users you have in your repo? I'm thinking of making the limit 100 as that's still pretty quick on my machine with my connection, but I know not all machines + connections will be as fast. |
Our org has 301 people and 102 teams, and it looks like we're trying to pull them all into that selector. |
I'm also seeing slow loading performance. I could swear it was faster at one point, but I haven't used this feature often enough to say for sure. |
The avatars could be the problem for you too @diminutivesloop. I opted for conservative and now only download all those avatars if there will be < 80 of them. The change will be available in the pre-release build tomorrow if you want to try it out! |
The new pre-release version is available it you want to try it out! |
It's SOOOOO much faster with the latest version!
Thanks @alexr00! 🎉 |
Excellent. Thanks @curtisgibby for testing this and providing logs so I could find the cause! |
When using the "Add Reviewers" component of the Description view, the autocomplete list of users shows all users available to request review from, but not any of the GitHub Organization teams that are available from the web view.
The text was updated successfully, but these errors were encountered: