1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-10 08:52:39 +01:00
Commit graph

1029 commits

Author SHA1 Message Date
David Cramer
0b833c2f02 Correct description and severity on Flake8 linter
Test Plan: do it live

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4274
2012-12-21 16:13:33 -08:00
David Cramer
4ca70d4e48 Add a flake8 linter
Summary: flake8 is the better maintained combination of pep8 and pyflakes

Test Plan: There's a test!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, jack

Differential Revision: https://secure.phabricator.com/D4082
2012-12-21 15:28:26 -08:00
epriestley
cffe1942ec Improve arc land recovery from issues with the remote.
Summary:
Fixes T2138.

  - When a pull fails, restore the original branch.
  - When a push fails, complain about it really loudly.

NOTE: No test plan for push yet since I'm not sure this is the right remedy, see T2138 for discsusion.

Test Plan:
  - Tested pull by changing "git pull" to "git xxpull" and running "arc land". Saw the pull fail and my original branch restored.

Reviewers: vrana, aran

Reviewed By: vrana

Maniphest Tasks: T2138

Differential Revision: https://secure.phabricator.com/D4265
2012-12-21 14:18:07 -08:00
vrana
f830b3bf3f Don't require clean working copy for arc diff path in SVN
Summary: If you are explicit then there is no need to ask you.

Test Plan:
  $ touch a
  $ arc diff
  $ arc diff a
  $ arc diff existing

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4186
2012-12-21 12:34:06 -08:00
vrana
c7c3f6a7f1 Update PEP8 to 1.3.4
Summary: Primarily to avoid false positives: http://pypi.python.org/pypi/pep8#id1

Test Plan:
  $ arc lint --engine ComprehensiveLintEngine externals/pep8/pep8.py # after uncommenting externals/ check

Saw 9 errors, haha.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4260
2012-12-21 11:34:40 -08:00
vrana
940d91d7b5 Speed up SVN discovery
Summary: `svn info` takes up to 10 seconds.

Test Plan: `arc diff` inside SVN repo and outside any repo.

Reviewers: nh, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4256
2012-12-20 18:10:37 -08:00
vrana
3f05751724 Don't amend commit with the same message in arc diff
Summary: It makes reflog ugly

Test Plan:
# `arc diff`, made error, saw "Message saved to".
# `arc diff`, didn't edit, didn't see "Message saved to".
# `arc diff`, edited, saw "Message saved to".

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4243
2012-12-20 12:18:05 -08:00
vrana
2d63d080df Make caching lint default
Summary: I use it couple of weeks without troubles.

Test Plan:
Added debug output and:

  $ arc lint
  $ arc lint --cache 0

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2036

Differential Revision: https://secure.phabricator.com/D4242
2012-12-20 12:16:19 -08:00
epriestley
ffdf44e298 Fix arc diff in SVN with paths
Summary:
Recently, in D4097 or one of the precursors I refactored this. However, when $rev is null parseBaseCommitArgument() throws ("This VCS does not support commit ranges."). Shield the call so it only happens if if $rev is nonempty (we still want to make the call, so "arc lint --rev x" on SVN will throw and inform the user that "--rev" is incorrect usage).

(@vrana, this was reported by FB and might be worth pushing.)

Test Plan: Ran "arc diff --preview <path>". Grepped for other parseBaseCommitArgument() callsites and verified they don't have similar issues.

Reviewers: vrana, btrahan, chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D4241
2012-12-20 11:12:30 -08:00
vrana
219dbe2374 Refuse writing to undeclared properties in workflows
Summary: I don't know how to not be strict here plus we (Arcanist developers) don't have access to user's error log.

Test Plan:
Undeclared `ArcanistDiffWorkflow::$console`, then:

  $ arc diff

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3607
2012-12-18 16:15:32 -08:00
vrana
2871d00f96 Allow checking out branches in arc branch
Summary:
The main added value is loading the branch name from revision.
Sometimes I know the revision ID but I don't know the branch name.

The missing piece is the starting point of the branch.
I was thinking about using `arc.land.onto.default` but we also need to get 'origin'.

This is also the last step of a simple workflow where underlying VCS is not abstracted away.
In future, we can implement this for other APIs.

Test Plan:
  $ arc branch new_branch
  $ arc branch new_branch
  $ arc branch D4168

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4170
2012-12-18 16:13:22 -08:00
Evan Priestley
955c5b8820 Merge pull request #62 from bonneval/bugfixUndefinedVariable
Bugfixed undefined variable $commit.
2012-12-18 05:38:05 -08:00
bonneval
551d036a89 Bugfixed undefined variable $commit. 2012-12-18 14:31:48 +01:00
Brennan Taylor
b5b1fd62dc Support http basic auth and related configuration
This adds the http basic auth username and password if found in the
configuration to the ConduitClient.

Reviewed by: epriestley
2012-12-17 18:06:54 -08:00
epriestley
b9fa71f7e1 Allow arc to be run in arcanist/ or libphutil/
Summary:
Currently, if you run `arc` in arcanist/ or libphutil/ and your PATH and on-disk configuration are set up so a different version of arc or libphutil are the ones that actually load, we fail with an exception like "running arcanist in a different copy of arcanist is not supported".

This causes problems for Harbormaster, since we'd like to be able to run 'arc' in a copy of libphutil/ and have it execute unit tests for that copy rather than failing abruptly. So, if we detect that we're in arcanist/ or libphutil/, execute 'arc' again with the same arguments but force it to load the working copy in place of either the 'arcanist/' or the 'libphutil/' that it decided to load.

This is pretty much horrible black magic.

Test Plan: Ran 'arc list --trace' inside copies of libphutil/ and arcanist/ outside of the normal include chain. Saw it detect these, emit a message, and re-execute itself correctly.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D4225
2012-12-17 16:35:03 -08:00
epriestley
6823706c76 Stop passsing bogus 'user' parameter to differential.createrevision
Summary: After D4191 this is a fatal.

Test Plan: Created this revision.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D4219
2012-12-17 14:41:27 -08:00
epriestley
716ce2482c Implement --everything, --json and --ugly flags for arc unit
Summary: Adds "arc unit --everything", which runs every available test, provided the test engine supports it. Also add JSON output.

Test Plan: Ran `arc unit --everything` in arcanist/, libphutil/ and phabricator/. Saw all tests run.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T2065

Differential Revision: https://secure.phabricator.com/D4214
2012-12-17 12:56:41 -08:00
epriestley
0bf5c3603c Refactor commit ranges and base commit handling
Summary:
See D4049, D4096.

  - Move commit range storage from Mercurial and Git APIs to the base API.
  - Move caching up to the base level.
    - Store symbolic name and resolved name separately, so we can re-resolve the correct commit from the symbolic name after dirtying caches.
  - Rename `supportsRelativeLocalCommit()` to `supportsCommitRanges()` (old name wasn't very good, and not consistent with new terminology like the `--base` flag).
  - Rename `getRelativeCommit()` and `setRelativeCommit()` to `getBaseCommit()` and `setBaseCommit()`.
  - Introduce `reloadCommitRange()` and call it from `reloadWorkingCopy()`.

I think this fixes the problem in D4049, and provides a general solution for the class of problems we're running into here, with D4096. Specifically:

  - We no longer get dirty caches, as long as you call reloadWorkingCopy() after changing the working copy (or call a method which calls it for you).
  - We no longer get order-of-parsing-things problems, because setBaseCommit() reloads the appropriate caches.
  - We no longer get nasty effects from calling `requireCleanWorkingCopy()` too early.

Test Plan: This is pretty far-reaching and hard to test. Unit tests; ran various arc commands. :/

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4097
2012-12-17 12:54:08 -08:00
epriestley
2ae0cb797d Remove ArcanistRepositoryAPI::setDefaultBaseCommit()
Summary:
This method is used in three cases:

  # For unit tests, to set the range to 'HEAD^' or '.^' in an agnostic way.
  # For "amend", to set the range to the commit to be amended (also 'HEAD^' or '.^').
  # For "patch" and "upgrade" so we don't fail just because there's an invalid "base" rule somewhere in the config when doing clean-working-copy tests.

For cases (1) and (2), introduce an "arc:this" rule to mean "the current commit". For case (3), remove the call; it is no longer necessary to check the commit range in order to do tests for the working copy state after D4095.

Test Plan: Ran unit tests, "arc upgrade", "arc patch", "arc amend".

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4096
2012-12-17 12:53:38 -08:00
epriestley
0e27dbfd17 Refactor getWorkingCopyStatus() into getUncommittedStatus() and getCommitRangeStatus()
Summary:
See discussion in D4049.

The getWorkingCopyStatus() method gets called from requireCleanWorkingCopy() in a lot of places, which triggers resolution of the base of the commit range. This is unnecessary; we do not need to examine the base commit in order to determine whether the working copy is dirty or not. This causes problems, in D4049 and elsewhere (we currently have a lot of fluff calls to setDefaultBaseCommit() in workflows which need to call requireCleanWorkingCopy() but do not ever use commit ranges, such as `arc patch`). This is mostly an artifact of SVN, where the "commit range" and "uncommitted stuff in the working copy" are always the same.

  - Split the method into two status methods: getUncommittedStatus() (uncommitted stuff in the working copy, required by requireCleanWorkingCopy()) and getCommitRangeStatus() (committed stuff in the commit range).
  - Lift caching out of the implementations into the base class.
  - Dirty the cache after we commit.

This doesn't do anything useful on its own and creates one caching problem (`commitRangeStatusCache` is not invalidated when the commit range changes because of `setBaseCommit()` or similar) but I wanted to break things apart here. I won't land it until there's a more complete picture.

This creates a minor performance regression in git and hg (we run less stuff in parallel than previously) but all the commands should be disk-bound anyway and the regression should be minor. It prevents a larger regression in `hg` in D4049, and lets us do less work to arrive at common error states (dirty working copy). We can examine perf at the end of this change sequence.

Test Plan: Ran unit tests, various `arc` commands.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4095
2012-12-17 12:53:28 -08:00
vrana
5f5392df8a Suggest upgrading Phabricator for unknown Conduit methods or parameters
Summary:
This allows using new methods without the need for bumping version number.

The usage is not neccessary because we already bumped the version number for this but I wanted to have a callsite.

Test Plan:
Made a typo in method name, then:

  $ arc lint --only-new 1

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4193
2012-12-14 18:16:01 -08:00
vrana
7133c76d37 Update PEP8
Test Plan:
Linted this Python file:

  def get_mapper_sets(
          compression=4.0,  # Currently works for most tables
          **kwargs):
      pass

Didn't see:

> E225 missing whitespace around operator

Reviewers: andrewjcg, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4172
2012-12-14 17:50:18 -08:00
durham
f476d1a0c5 add arc land hg support for landing with nested branches
Summary:
This changes arc land's hg support to allow you to land a branch or bookmark that has nested branches/bookmarks.  Example:

      // Initial state:
      // -a--------b  master
      //   \
      //    w--x  mybranch
      //        \--y  subbranch1
      //         \--z  subbranch2
      //
      // arc land --branch mybranch --onto master :
      // -a--b--wx  master
      //          \--y  subbranch1
      //           \--z  subbranch2

Test Plan:
Created several repos like in the summary and ran 'arc land' and 'arc land --keep-branch'. Did this with both bookmarks and named branches. Scenarios tested:

- mybranch having no child commits
- mybranch having a child branch with several commits
- mybranch having two child branches
- mybranch having a child branch which has two more child branches

No code was added outside of a "if ($this->isHg)" so I didn't run git arc land.

Reviewers: epriestley, dschleimer, bos, sid0

Reviewed By: dschleimer

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4089
2012-12-14 11:23:44 -08:00
epriestley
2d7224f0e3 Add an experimental "--mergeup" flag to use a less volatile merge strategy in "arc land"
Summary:
See chatlog from https://secure.phabricator.com/chatlog/channel/%23phabricator/?at=37257

Basically, the install's developers sometimes use "git merge master" instead of "git rebase master" to pull changes from master into their branch. When we run "git rebase master" at the end, this creates a lot of conflicts.

Instead, optionally run "git merge master". This should be equivalent in our case (where we always rebase) and much better in their case (where they sometimes merge).

We're both going to use it for a bit and see if it creates problems. If it improves the "sometimes-merge" workflow without affecting the "always rebase" workflow, we can make it a default. If it improves theirs but damages ours, we can keep it a flag/option. If it's just terrible, we can figure out something else.

Test Plan:
Ran `arc land --mergeup --trace <feature> --keep-branch --hold`, see P624 for output. Observed merge update strategy and general success.

Also verified the conflict:

  $ arc land --mergeup --merge
  Usage Exception: Arguments '--mergeup' and '--merge' are mutually exclusive: The --merge strategy does not update the feature branch.

Reviewers: btrahan, vrana, DurhamGoode

Reviewed By: btrahan

CC: aran, keir

Differential Revision: https://secure.phabricator.com/D4080
2012-12-13 14:11:52 -08:00
vrana
02694d73ab Simplify code by not passing local data to methods 2012-12-12 14:21:06 -08:00
vrana
48077c80f2 Use repository info from arcanist.projectinfo if available
Test Plan: Added a debug output there and ran.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4147
2012-12-10 15:18:52 -08:00
vrana
c68a048213 Support git svn dcommit in arc land
Test Plan:
  $ arc land # in Git SVN repository

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4123
2012-12-10 12:28:44 -08:00
Aviv Eyal
7b2ce3a987 Add arc browse workflow
Summary:
arc browse will open a browser to the Diffusion view of a file. Convenient if you like
Diffusion for reading source. Naturally, it fixes relative filenames.

Combined with git-grep it can be an easy replacement for server-side search functions.

Test Plan:
use feature with and without 'browser' configured.

I've only tested this on Linux, because that's all I have right now, but the principle is sound.

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4127
2012-12-09 14:10:21 -08:00
vrana
65e3583235 Reuse code in close-revision workflow
Test Plan: Will test it by closing this revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4122
2012-12-07 18:22:23 -08:00
epriestley
c43d627cf2 Remove ArcanistSubversionAPI::hasMergeConflicts()
Summary: No callsites, redundant with getMergeConflicts().

Test Plan: Grepped for callsites.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4093
2012-12-06 12:20:02 -08:00
Edward Speyer
ecdb885236 Quote author parameter
Summary:
Fixes the exception:

  Exception
  Command 'git commit -a --author=Whatever Long Name <whatever@email.com> -F -' failed with error #1

Test Plan: Tested with full git name

Reviewers: epriestley, aurelijus

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3979
2012-12-05 15:45:54 -08:00
Xingyao Ye
6c3418a0e8 Introduce "arc diff --plan-changes"
Summary:
Allow authors to publish a new or updated revision to Phabricator
without requesting code reviews. The revision will have status
"Needs Revision" instead of "Needs Review".

In order to avoid a change of Conduit API, this is done by adding
a comment with the "plan changes" action immediately after the
revision is published.

Test Plan:
Using my local repository, run "./bin/arc diff --plan-changes"
Check the resulting diff in Phabricator.

Reviewers: vrana

Reviewed By: vrana

CC: aran, epriestley

Maniphest Tasks: T2024

Differential Revision: https://secure.phabricator.com/D4084
2012-12-05 14:04:11 -08:00
epriestley
ba1a17ac31 Fix a fatal for git uncommitted changes in new arc projects
Summary:
If you run `arc diff` in a repository which:

  - has uncommitted or untracked changes; and
  - has a .arcconfig with a never-before-seen project ID;
  - we fatal: http://pastebin.com/raw.php?i=ykpfr4MT

This patch is a bit iffy, open to alternatives. The "right" patch is probably an `arcanistproject.query` which behaves more sensibly.

I return array() directly since we'll later create the project.

Test Plan: Ran `arc diff` in a repository with untracked files or uncommitted changes and a new project ID.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4057
2012-12-05 09:20:15 -08:00
vrana
209ffba8c0 Bump Conduit client version number
Summary: Should have been part of D3934.

Test Plan:
  $ arc diff

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4076
2012-12-03 18:09:45 -08:00
durham
aada1440ef Add hg support for arc land
Summary:
Makes arc land support hg repositories. Both bookmarks and named branches can be landed.  For the most part all the arc land options work, but there are a few caveats:

- bookmarks can only be landed on bookmarks
- branches can only be landed on branches
- landing a named branch with --merge creates a commit to close the branch before the merge.
- since mercurial doesn't start with a default master bookmark, landing a bookmark requires specifying --onto or setting arc.land.onto.default

Test Plan:
Tested arc land with all permutations of --merge, --keep-branch on both bookmark branches and named branches.  Also tested --hold, --revision, --onto, --remote.

See https://secure.phabricator.com/P619

Also tested git arc land with --merge and --keep-branch.

Reviewers: dschleimer, sid0, epriestley, bos

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4068
2012-12-03 17:59:12 -08:00
vrana
5025c06f3d Introduce arc lint --only-new 1
Test Plan:
  $ arc lint src/workflow/ArcanistLintWorkflow.php
  $ arc lint --only-new 1 src/workflow/ArcanistLintWorkflow.php

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2038

Differential Revision: https://secure.phabricator.com/D3934
2012-12-03 17:21:17 -08:00
epriestley
886c1721dd Restore order of operations after D4056
Summary:
See discussion in D4056. In `findRevision()` we call `loadWorkingCopyDifferentialRevisions()`. However, this method depends upon the state of the working copy, because the end of the commit range it examines is HEAD. Prior to D4056 we checked out the target branch before calling `findRevision()`; after D4056 we call it earlier.

This isn't problematic in the `arc land` case, but in the `arc land <branch>` case it means we may fail to identify a revision, or identify the wrong revision, because HEAD isn't where we expect it to be.

Instead, unconditionally check out the target branch before finding the revision.

See <http://dl.dropbox.com/u/116385/Slingshot/Pictures/Screen%20Shot%202012-12-03%20at%203.43.45%20PM.png> for a transcript of the issue.

Test Plan: Reproduced issue as per link above. Ran `arc land --keep-branch --hold somebranch` successfully after this patch.

Reviewers: DurhamGoode, zeeg

Reviewed By: DurhamGoode

CC: aran

Differential Revision: https://secure.phabricator.com/D4072
2012-12-03 16:31:45 -08:00
vrana
e8eacb6ae3 Allow setting lint cache granularity
Summary:
We can add `GRANULARITY_DIRECTORY` and `GRANULARITY_REPOSITORY` later.
Repository granularity may use current commit + changes.
Directory would need to use hashes of all files in dir which would be quite expensive.

Test Plan:
  $ echo '<?php class A extends B {}' > A.php
  $ arc lint --cache 1
  $ arc lint --cache 1
  $ echo '<?php class B {}' > B.php
  $ arc lint --cache 1
  $ arc lint --cache 1
  $ rm B.php
  $ arc lint --cache 1
  $ arc lint --cache 1

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4021
2012-12-03 15:30:06 -08:00
David Cramer
0d1d04e434 canRun should return false on scala linter instead of raising an exception
Summary:
This was causing the following error in environments that didnt have scala configured:

Some linters failed:
   - ArcanistScalaSBTLinter: ArcanistUsageException: This directory does not appear to be maintained by SBT, as we can't seem to find a working build file (project/Build.scala or build.sbt).

Test Plan: .

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4070
2012-12-03 15:25:28 -08:00
durham
3602d2aa15 Break arc land run() into smaller functions
Summary:
Refactor the arc land code into several
functions so it's easier to maintain. This is in
preparation for adding hg support to arc land
in my next commit.

Without this refactor, adding hg support makes the run()
function too big.

Test Plan:
Set up a git repo and clone with a branch scenario.
Example: https://secure.phabricator.com/P614
Ran and verified:
arc land
arc land --keep-branch
arc land --merge
arc land --merge --keep-branch
arc land --hold
arc land --revision <another phabricator rev>

Reviewers: epriestley

Reviewed By: epriestley

CC: dschleimer, sid0, aran, Korvin

Differential Revision: https://secure.phabricator.com/D4056
2012-12-03 13:03:13 -08:00
Ricky Elrod
b549f565c9 Add a very basic Scala (SBT) linter.
Summary:
SBT is the most common Scala buildsystem. This adds an extremely basic and
slightly horrible linter to check SBT's output for warnings and errors.

Test Plan:
Tested this with a Scala project I've been working on for some time.
It seemed relatively sane.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4064
2012-12-03 10:04:18 -08:00
vrana
6cb8d483b2 Improve getting Git author
Summary:
It has one more than source, see http://www.kernel.org/pub/software/scm/git/docs/git-commit-tree.html#_commit_information.
Also 'user.name' may not be set at all.
Also `git config` returns non-zero for non-existing value.

Test Plan:
  var_dump($api->getAuthor());

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4053
2012-11-30 11:09:31 -08:00
vrana
9917c1f06a Provide a method for getting Git SVN revision
Summary: I plan to use it in `save_lint.php`.

Test Plan:
  $api->getUnderlyingWorkingCopyRevision(); // In Git SVN repo

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4054
2012-11-30 11:06:23 -08:00
vrana
c12dee997d Prefer Mercurial and Git over Subversion in discovering repository
Summary: `svn info` is the slowest command for discovering repository (up to 300 ms) and Subversion is probably the least used repository type with Arcanist. Let's discover it last.

Test Plan:
  $ arc lint --trace

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4039
2012-11-27 13:48:31 -08:00
epriestley
db25adb069 Parse "--help" correctly in arc
Summary:
We delegate parsing to `PhutilArgumentParser` but then skip the parts of the code which do something with this flag.

In particular, `arc --help list` just runs `arc list`.

See https://github.com/facebook/arcanist/issues/58

Test Plan: Ran `arc --help`, `arc list --help`, `arc --help list`, `arc --help help --help`. Got expected help in all cases.

Reviewers: vrana, chad, btrahan

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D4023
2012-11-22 08:36:28 -08:00
vrana
cd7f86d380 Set cache version per linter
Summary: Also delete cache with `--cache 0`.

Test Plan:
  $ arc lint --lintall --cache 1
  $ cat .git/arc/lint-cache.json
  $ arc lint --lintall --cache 1 # with debug output

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2036

Differential Revision: https://secure.phabricator.com/D4013
2012-11-21 17:22:06 -08:00
epriestley
d3afa9cc31 Fix some arcanist warnings
Summary:
  - Rename some very old variables.
  - Wrap some contributed lines.

Test Plan: `arc lint` / `arc unit`. Viewed a diff in an uncacheable mode to verify intraline behavior.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4018
2012-11-21 17:16:52 -08:00
vrana
fc2cd63739 Fix getting changed files in Git 2012-11-21 15:57:06 -08:00
vrana
10bcbf1f4b Emit advice for TODO in Phutil linter
Summary:
Let's try this.
It may be useful to see this together in Differential overview.

Test Plan: Linted a file with TODO.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T744

Differential Revision: https://secure.phabricator.com/D4014
2012-11-21 15:40:58 -08:00
vrana
3cfd58c29c Allow caching lint results
Summary:
There is one big decision: How to get linters version.
I've created `getCacheVersion()` which is supposed to be bumped every time engine or any linter is changed.
This is not very nice but the other alternative (detect this automatically) seems worse:

- The engine may be outside repo and may or may not be under version control so getting its version through something like `git log` may not be even possible.
- If it is in the same repo then every rebase will obsolete the whole cache.

Even though bumping the version manually is PITA I still think it's a better solution.

Test Plan:
  $ time arc lint --cache 1
  # verified file
  $ arc arc lint --cache 1
  # added some debug output to see the cached results
  # also observed better time (.57 s instead of 2.19 s)

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2036

Differential Revision: https://secure.phabricator.com/D4006
2012-11-21 13:21:10 -08:00