Page MenuHomePhorge

Implements arc work for git workflow
Needs ReviewPublic

Authored by nib on Thu, Feb 13, 05:07.

Details

Summary

Makes arc work [task] work with git. Creates git branch with task id and task name.

Test Plan

Try arc work T12345

Diff Detail

Repository
rARC Arcanist
Branch
T15993-support-for-arc-work-t12345-(workontask-workflow) (branched from master)
Lint
Lint Errors
SeverityLocationCodeMessage
Errorsrc/work/ArcanistWorkEngine.php:208XHP1PHP Syntax Error!
Errorsrc/work/ArcanistWorkEngine.php:227MERGECONFLICT1Unresolved merge conflict
Warningsrc/work/ArcanistWorkEngine.php:213TXT3Line Too Long
Warningsrc/work/ArcanistWorkEngine.php:250TXT3Line Too Long
Unit
Test Failures
Build Status
Buildable 1734
Build 1734: arc lint + arc unit

Unit TestsFailed

TimeTest
72 msPhutilLibraryTestCase::testEverythingImplemented
EXCEPTION (PhutilMissingSymbolException): Failed to load symbol "ArcanistWorkEngine" (of type "class or interface"). The symbol map for library 'arcanist' (at '/Users/nick/Code/PhpstormProjects/arcanist2/src') claims this class or interface is defined in 'work/ArcanistWorkEngine.php', but loading that source file did not cause the class or interface to become defined.
167 msPhutilLibraryTestCase::testLibraryMap
EXCEPTION (XHPASTSyntaxErrorException): work/ArcanistWorkEngine.php: XHPAST Parse Error: syntax error, unexpected '<' on line 208 #0 /Users/nick/Code/PhpstormProjects/arcanist2/src/moduleutils/PhutilLibraryMapBuilder.php(63): PhutilLibraryMapBuilder->analyzeLibrary() #1 /Users/nick/Code/PhpstormProjects/arcanist2/src/__tests__/PhutilLibraryTestCase.php(29): PhutilLibraryMapBuilder->buildMap()
29 msPhutilLibraryTestCase::testMethodVisibility
EXCEPTION (PhutilMissingSymbolException): Failed to load symbol "ArcanistGitWorkEngine" (of type "class or interface"). The symbol map for library 'arcanist' (at '/Users/nick/Code/PhpstormProjects/arcanist2/src') claims this class or interface is defined in 'work/ArcanistGitWorkEngine.php', but loading that source file did not cause the class or interface to become defined.

Event Timeline

nib requested review of this revision.Thu, Feb 13, 05:07

Thanks! I share a comment on the fly, maybe more comments soon after a test :)

src/work/ArcanistWorkEngine.php
278

Multi-byte support for emojis

aklapper added inline comments.
src/work/ArcanistWorkEngine.php
201

This should not be indented

aklapper requested changes to this revision.Sun, Feb 16, 16:51

Setting Request Changes per the small review comments so far (as I am looking at https://we.phorge.it/differential/query/all/ and it helps to see correct statuses)

This revision now requires changes to proceed.Sun, Feb 16, 16:51

Hi @nib - still thanks and you are welcome in updating this patch using something like this:

arc patch D25872
(edits)
arc diff

If you can't, we can eventually help. Thanks!

  • Fixes indents, adds emoji support, improves REGEX

I've tested this and I'm impressed. You probably have changed my life.

Since the original workflow was completely borked I supposed this is the minimal viable product. But note that running the command twice causes this:

STDERR
fatal: a branch named 'T15187-blabla' already exists
  1. Probably in this specific case it should just try to checkout there without creating the branch (?). I also don't think it should behave like arc patch that creates something different.
  1. Also, what should happen if the title changes a bit and if I run it again? Maybe it should detect tasks starting with Tsomething and use that?

What do you think about? Note that I'm still impressed and happy as-is lol

nib marked 2 inline comments as done.

Fixes error when invoking arc work 2nd time on task id

  • Handles existing branches
  • Fixes indents, adds emoji support, improves REGEX
  • Fix indents, adds emoji support, improve REGEX
  • Detect existing branch for task

Hopefully the latest change will work for you. Now it checks existing git branches matching "T12345*" and if any exist, it will git checkout the first one, else it will create it from the task name.
So that should allow you to change the task title, and to use arc work T12345 as many times as you want without error.

I've tested this and I'm impressed. You probably have changed my life.

Since the original workflow was completely borked I supposed this is the minimal viable product. But note that running the command twice causes this:

STDERR
fatal: a branch named 'T15187-blabla' already exists
  1. Probably in this specific case it should just try to checkout there without creating the branch (?). I also don't think it should behave like arc patch that creates something different.
  1. Also, what should happen if the title changes a bit and if I run it again? Maybe it should detect tasks starting with Tsomething and use that?

What do you think about? Note that I'm still impressed and happy as-is lol

@aklapper Would it make sense to move the branch-naming lines into a function in class ArcanistGitWorkEngine, since it the branch-naming is git-specific? E.g.

public function buildBranchName($task_ref) {
  $task_name = $task_ref->getName();
  // regex, preg_replace, trim, etc
  return $branch_name;
}
In D25872#23861, @nib wrote:

@aklapper Would it make sense to move the branch-naming lines into a function in class ArcanistGitWorkEngine, since it the branch-naming is git-specific? E.g.

public function buildBranchName($task_ref) {
  $task_name = $task_ref->getName();
  // regex, preg_replace, trim, etc
  return $branch_name;
}

Probably yes so it's easier with Subversion and Mercurial in the future (?)

Probably something with "for task" in the name like buildBranchNameForTask($task_ref)

src/work/ArcanistWorkEngine.php
201

And we can remove this newline <3 ihih