Problems with the reviewers list


#1

I wanted to add another developer but a weird thing happened.

  1. Logged into the Dashboard and navigated to the repo of interest (hgrecco/pint)
  2. Opened the Settings Tab. The Reviewers list was fine but there was no text box to add a new one.
  3. Opened GitHub in another browser tab, repo > Settings > App and integrations > Bors > Configure
  4. Everything seemed fine, went back to the Bors Dashboard, refresh the page and the Reviewers list was empty (except for me). The text box was still not there.

#2

In the Settings tab, you can set Synchronize to Admin, Push, Pull, or None. The text box is hidden unless you have it set to None.

The Synchronize setting allows you to manage the collaborators in GitHub and have it synced with bors.


#3

Thanks a lot for the clarification. This was not clear to me and I destroyed my list. Maybe a warning could be added when changing from Admin, Push, Pull, or None. Or at least make it less easy to do. In any case, thanks a lot for the explanation and for Bors


#4

I was just investigating some weirdness with our permissions and noticed the changes for #411. I believe there’s some surprising UX with the manner that PR was implemented. Specifically, the change to automatically remove users who don’t have the specified permission combined with defaulting the sync option to admin means that most projects will witness the behavior @hgrecco describes.

I understand why this is how it was built, each decision makes perfect sense in isolation: sync is generally expected to both add and remove, and the previous default was admin before there was an option for anything else. However, the end result is that any manually-curated lists of reviewers got clobbered. It looks like that’s what happened to us when we upgraded our hosted version.

It may be way past the point of diminishing returns on this by now, but perhaps switching the default to “None” would make for fewer surprises?


#5

Yeah, what was flubbed was the migration. Old repositories should’ve been left at None as their default, with new repositories being set to Admin mode. Unfortunately, for awhile, the feature was in the repository, not fully implemented, and was still having new repositories created with the Admin mode.

What an embarrassing, and totally preventable, mistake.


#6

Hi @notriddle, hope I didn’t come across as too annoying, just wanted to describe it clearly so that it was written down. Can’t do everything perfectly. Keep up the good work!