Skip to content

Stable reviewer roulette

Sean McGivern requested to merge stable-reviewer-roulette into master

Change reviewer roulette to always pick the same reviewers for the same branch name. We do this by:

  1. Making the branch name 'canonical' across CE and EE by stripping a leading 'ce-' or 'ee-' and a trailing '-ce' or '-ee'. If people are following our branch naming guidelines, this should give the same branch name in both repos.
  2. Converting the branch name to a stable integer by taking the integer form of its MD5.
  3. Passing that integer as a seed to Ruby's Random class, which 'may be used to ensure repeatable sequences of pseudo-random numbers between different runs of the program' (from the Ruby documentation).

The upshot is that the same branch name (in CE and EE) should always pick the same reviewers, and those should be evenly distributed across the set of possible reviewers due to the use of MD5.

Again, I have a test script:

require 'ffaker'

class Foo
  include Gitlab::Danger::Helper
end

def spin(team, project, category, branch_name)
  # Strip leading and trailing CE/EE markers
  canonical_branch_name = branch_name.gsub(/^[ce]e-/, '').gsub(/-[ce]e$/, '')
  rng = Random.new(Digest::MD5.hexdigest(canonical_branch_name).to_i(16))

  reviewers = team.select { |member| member.reviewer?(project, category) }
  traintainers = team.select { |member| member.traintainer?(project, category) }
  maintainers = team.select { |member| member.maintainer?(project, category) }

  # TODO: filter out people who are currently not in the office
  # https://gitlab.com/gitlab-org/gitlab-ce/issues/57652
  #
  # TODO: take CODEOWNERS into account?
  # https://gitlab.com/gitlab-org/gitlab-ce/issues/57653

  # Make traintainers have triple the chance to be picked as a reviewer
  reviewer = (reviewers + traintainers + traintainers).sample(random: rng)
  maintainer = maintainers.sample(random: rng)

  [reviewer.username, maintainer.username]
end

def random_branch_name
  FFaker::Filesystem.file_name
end

FFaker::Random.seed = 123
team = Foo.new.project_team
results = Hash.new(0)

10_000.times do
  reviewer, maintainer = spin(team, 'gitlab-ce', 'backend', random_branch_name)

  results[reviewer] += 1
  results[maintainer] += 1
end

results.sort_by(&:last).reverse.each do |username, picked|
  puts "#{username}: #{picked}"
end; nil

This should output the same for you as it does for me, because we seed the branch names too!

dzaporozhets: 799
mkozono: 797
grzesiek: 794
godfat: 793
DouweM: 788
nick.thomas: 773
tkuah: 764
stanhu: 761
ayufan: 759
jprovaznik: 758
jameslopez: 757
dbalexandre: 754
smcgivern: 751
splattael: 744
ashmckenzie: 741
rspeicher: 739
jarka: 738
rymai: 735
reprazent: 729
mayra-cabrera: 713
mdelaossa: 273
rdavila: 269
vsizov: 260
theoretick: 257
oswaldo: 255
ifarkas: 255
brytannia: 248
engwan: 248
felipe_artur: 247
rpereira2: 243
toon: 239
rossfuhrman: 236
jamedjo: 235
fjsanpedro: 229
matteeyah: 226
brodock: 226
vzagorodny: 222
zj: 220
markglenfletcher: 219
dosuken123: 206

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/57766.

Merge request reports