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)
Wed, Mar 22, 08:37
Unknown Object (File)
Fri, Mar 17, 03:36
Unknown Object (File)
Thu, Mar 16, 04:02
Unknown Object (File)
Wed, Mar 8, 06:01
Unknown Object (File)
Wed, Mar 8, 06:01
Unknown Object (File)
Wed, Mar 8, 06:01
Unknown Object (File)
Wed, Mar 1, 22:27
Unknown Object (File)
Wed, Mar 1, 22:27

Details

Summary

WIP

Make arcanist follow the zero one infinity rule

Test Plan

WIP

Diff Detail

Repository
rARC Arcanist
Lint
Lint Skipped
Unit
Tests Skipped

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.

src/workflow/ArcanistPatchWorkflow.php
222

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

235

This can be just $proposed_suffix++.

src/workflow/ArcanistPatchWorkflow.php
221

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

222

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.Sun, Mar 19, 14:15