Re: RBTools Ticket #4975: `rbt post` should infer default reviewers from Google/Gerrit-style OWNERS, GitLab-/GitHub-style CODEOWNERS files
-- To reply, visit https://hellosplat.com/s/beanbag/tickets/4975/ -- New update by beasleyr-vmw For Beanbag, Inc. > RBTools > Ticket #4975 Reply: > We've had a long-standing goal to change up how default reviewers work. Right now they're regex-based, as you noted. The plan though is to let admins define rules using our Conditions support (which we use for service integrations). These would allow for anything from regex rules to "look up default reviewers from this file/URL" to "ask this extension/webhook for reviewers". This is very encouraging news. I look forward to seeing it appear in the final product. :) With that in mind, I believe we can close/resolve this public ticket. (I'm not sure if users can resolve tickets or if that's limited to Splat/project admins.) -- You received this message because you are subscribed to the Google Groups "reviewboard-issues" group. To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard-issues+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/reviewboard-issues/20220816164344.11083.65381%40ip-10-1-54-209.ec2.internal.
RBTools Ticket #4975: `rbt post` should infer default reviewers from Google/Gerrit-style OWNERS, GitLab-/GitHub-style CODEOWNERS files
-- To reply, visit https://hellosplat.com/s/beanbag/tickets/4975/ -- New ticket #4975 by beasleyr-vmw For Beanbag, Inc. > RBTools Status: New Tags: Priority:Medium, Type:Enhancement -- `rbt post` should infer default reviewers from Google/Gerrit-style OWNERS, GitLab-/GitHub-style CODEOWNERS files == # What version are you running? Review Board 3.0.18 RBTools 2.0 (Python 3.6.8) # Describe the enhancement and the motivation for it. ## Enhancement `rbt post` optionally takes over the responsibility of determining which users/groups a review request should be sent to for a given change. The mapping of source files to users/groups can be configured using one or more of the following de facto standards. - GitHub CODEOWNERS [link](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners) - GitLab CODEOWNERS [link](https://docs.gitlab.com/ee/user/project/code_owners.html) - Google/Gerrit OWNERS [link](https://gerrit.googlesource.com/plugins/find-owners/+/master/src/main/resources/Documentation/syntax.md) I suggest implementing support for all 3 types. RBTools can either probe to determine which style is in use for a given repo, or it can let users choose which style applies their repo by setting the appropriate flag in `.reviewboardrc`. ## Motivation Defining default reviewers in large repositories is very difficult, because Review Board's default reviewers configuration is based on regular expressions that require superuser privileges to edit. This prevents users from fine-tuning default reviewer configurations, because (a) the desired behavior needs to somehow be able to be captured by one or more regexps, and (b) each tweak to the set of regexps requires a support ticket. Using OWNERS/CODEOWNERS would push this configuration out from the RB server to the actual project repository where changes in ownership can be managed by end developers instead of an admin. # Please provide any additional information below. I filed this under RBTools (as opposed to Review Board) for two reasons: 1. `rbt post` already supports `--target-groups` and `--target-people`, so I figured it'd make sense for `rbt post` to infer appropriate values based on OWNERS files. 2. Users can upgrade RBTools at their convenience, and so folks can easily adopt this feature w/o waiting for central admins to upgrade central infra first. I may be able to allocate cycles to contribute patches for this. -- -- You received this message because you are subscribed to the Google Groups "reviewboard-issues" group. To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard-issues+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/reviewboard-issues/20220809202031.28969.10560%40ip-10-1-54-209.ec2.internal.
RBTools Ticket #4806: Parallelize diff generation when SCM is Perforce (or any centralized SCM)
-- To reply, visit https://hellosplat.com/s/beanbag/tickets/4806/ -- New ticket #4806 by beasleyr-vmw For Beanbag, Inc. > RBTools Status: New Tags: Priority:Medium, Type:Enhancement -- Parallelize diff generation when SCM is Perforce (or any centralized SCM) == # What version are you running? RBTools 1.0.1 # Describe the enhancement and the motivation for it. Perforce is a client-server SCM system. Each file diff generated by `rbt post` incurs latency of ~1s, and a single changeset of N files requires N `p4 print` callouts in serial. So, a change with ~25 files burns ~30s generating diffs. It'd be great if `rbt post`, instead of generating diffs serially, perhaps tossed all of the `p4 print` callouts (or the entire "writing X to tmpfile; p4 print; diff tmpfile $output" sequence) into a threadpool (default size: ncpus?) in order to parallelize this task. # Please provide any additional information below. -- -- You received this message because you are subscribed to the Google Groups "reviewboard-issues" group. To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard-issues+unsubscr...@googlegroups.com. To post to this group, send email to reviewboard-issues@googlegroups.com. Visit this group at https://groups.google.com/group/reviewboard-issues. For more options, visit https://groups.google.com/d/optout.
Review Board Ticket #4572: X-ReviewBoard-Diff-For headers are incomplete
--
To reply, visit https://hellosplat.com/s/beanbag/tickets/4572/
--
New ticket #4572 by ryan.beasley
For Beanbag, Inc. > Review Board
Status: New
Tags: Priority:Medium, Type:Defect
--
X-ReviewBoard-Diff-For headers are incomplete
==
# What version are you running?
2.0.20
# What's the URL of the page containing the problem?
n/a
# What steps will reproduce the problem?
1. Post a review where the diff includes plain edits to an existing file (i.e.
does not involve creating, copying, or moving).
2. Upon receipt of generated e-mail, examine X-ReviewBoard-Diff-For headers.
3. Observe that X-ReviewBoard-Diff-For headers appear only for files named in
the diffset which were created/moved. Edited files are not named.
# What is the expected output? What do you see instead?
- I expect X-ReviewBoard-Diff-For headers to name every file (up to the 8K
limit) edited in a diffset.
# What operating system are you using? What browser?
- n/a
# Please provide any additional information below.
If I'm reading the code correctly, this appears to be a bug from the original
implementation which continues to the present. Any filediff for which none of
`filediff.{deleted,copied,moved,is_new}` applies fails to generate an
X-ReviewBoard-Diff-For header.
>From release-2.0.20 (reviewboard/notifications/email.py):
```
601 if latest_diffset:
602 modified_files = set()
603
604 for filediff in latest_diffset.files.all():
605 if filediff.deleted or filediff.copied or filediff.moved:
606 modified_files.add(filediff.source_file)
607
608 if filediff.is_new or filediff.copied or filediff.moved:
609 modified_files.add(filediff.dest_file)
610
611 for filename in modified_files:
612 headers.appendlist('X-ReviewBoard-Diff-For', filename)
```
And from HEAD (reviewbaord/notifications/email/message.py):
```
181
182 if latest_diffset:
183 modified_files = set()
184
185 for filediff in latest_diffset.files.all():
186 if filediff.deleted or filediff.copied or filediff.moved:
187 modified_files.add(filediff.source_file)
188
189 if filediff.is_new or filediff.copied or filediff.moved:
190 modified_files.add(filediff.dest_file)
191
192 # The following code segment deals with the case where the client
adds
193 # a significant amount of files with large names. We limit the
number
194 # of headers; when more than 8192 characters are reached, we stop
195 # adding filename headers.
196 current_header_length = 0
197
198 for filename in modified_files:
...
212 headers.appendlist('X-ReviewBoard-Diff-For', filename)
```
--
--
You received this message because you are subscribed to the Google Groups
"reviewboard-issues" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to reviewboard-issues+unsubscr...@googlegroups.com.
To post to this group, send email to reviewboard-issues@googlegroups.com.
Visit this group at https://groups.google.com/group/reviewboard-issues.
For more options, visit https://groups.google.com/d/optout.
