Page MenuHomePhorge

Allow an infinite number of arcpatch_DXXXX_X branches to be created
Needs RevisionPublic

Authored by ncollins on Nov 12 2021, 20:23.
Referenced Files
Unknown Object (File)
Sat, Jul 13, 02:53
Unknown Object (File)
Sat, Jul 13, 02:53
Unknown Object (File)
Sat, Jul 13, 02:44
Unknown Object (File)
Fri, Jul 12, 12:41
Unknown Object (File)
Fri, Jul 12, 12:40
Unknown Object (File)
Fri, Jul 12, 12:40
Unknown Object (File)
Fri, Jul 12, 06:42
Unknown Object (File)
Thu, Jul 11, 18:59




Make the Arcanist arc patch command to follow the "zero one infinity" rule.

Before this change, you cannot run the same command more than 4 times.

Closes T15173

Test Plan


Diff Detail

rARC Arcanist
Lint Passed
Tests Passed
Build Status
Buildable 118
Build 118: arc lint + arc unit

Event Timeline

Although there's merit to the "zero, one, infinity" rule, it might not be the best option here. If something goes wrong and $err happens to always be falsy, this will end up in an infinite loop instead of giving a clear error message. There is probably a reasonable finite value (that's greater than 4) which can be chosen as the limit to the number of attempts.


Will this result in "{$base_name}_" rather than "{$base_name}" as the first proposed branch name?


This can be just $proposed_suffix++.


I think the linter would've requested there to be a space between while and open paren


Yea I think this would result in the first non-collision attempt always being suffixed with underscore. The naive approach here would be

if ($proposed_suffix == null) {
  $proposed_name = $base_name;
} else {
  $proposed_name = "{$base_name}_{$proposed_suffix}";

I also think that having the first branch called as arcpatch-D123_ (with trailing underscore) as default could be considered a breaking change by some users. I think this can and it should be avoided.

I see that you marked this as "Work in progress." I don't know how to highlight this fact, so I'm marking this as "Request Changes" (also because some changes were requested). Sorry if this puts a red mark, but I like this proposal of infinite executions of arc patch.

This revision now requires changes to proceed.Mar 19 2023, 14:15