Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
branch: master
Commits on Jul 1, 2015
  1. @epriestley

    Raise a more tailored error message when a third-party test engine re…

    epriestley authored
    …turns bad results
    
    Summary: Fixes T8714. When a test engine isn't returning the correct result type, shift suspicion onto it.
    
    Test Plan: Faked error, got exception blaming test engine.
    
    Reviewers: joshuaspence, btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T8714
    
    Differential Revision: https://secure.phabricator.com/D13486
Commits on Jun 25, 2015
  1. @epriestley

    Don't fail lint builds for "advice"; make lint/unit upload failures l…

    epriestley authored
    …ouder
    
    Summary:
    Ref T8670. Ref T8657.
    
      - When lint only has advice (no warnings/errors), consider it a "passing" build.
      - Be a little louder about `sendmessage` calls failing because this stuff is not totally broken and that makes T8670-related things easier to catch/fix.
    
    Test Plan: Created this revision.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T8657, T8670
    
    Differential Revision: https://secure.phabricator.com/D13436
  2. @joshuaspence

    Return `$this` from setter methods

    joshuaspence authored
    Summary: Return `$this` from a bunch of setter methods for consistency. See also D13422.
    
    Test Plan: Eyeball it.
    
    Reviewers: #blessed_reviewers, epriestley
    
    Reviewed By: #blessed_reviewers, epriestley
    
    Subscribers: epriestley, Korvin
    
    Differential Revision: https://secure.phabricator.com/D13423
Commits on Jun 24, 2015
  1. @joshuaspence

    Ensure that a space is used after a catch token

    joshuaspence authored
    Summary: For consistency, a single space should separate `catch` and the following parenthetical expression.
    
    Test Plan: Added test case.
    
    Reviewers: epriestley, #blessed_reviewers
    
    Reviewed By: epriestley, #blessed_reviewers
    
    Subscribers: epriestley, Korvin
    
    Differential Revision: https://secure.phabricator.com/D13389
Commits on Jun 23, 2015
  1. @epriestley

    Try to ship lint and unit results to Harbormaster autotargets

    epriestley authored
    Summary:
    Ref T8095. Before uploading lint/unit results in the old way, try to ship them to autotargets.
    
    If we can query and upload data to autotargets, do so, and then skip the older style uploads.
    
    Test Plan: Used `arc diff` to ship data up via Harbormaster.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T8095
    
    Differential Revision: https://secure.phabricator.com/D13381
Commits on Jun 19, 2015
  1. @joshuaspence

    Fix arcanist shell completion

    joshuaspence authored
    Summary: Fixes T8560.
    
    Test Plan: Ran `arc shell-complete` outside of a working copy.
    
    Reviewers: avivey, #blessed_reviewers, epriestley
    
    Reviewed By: avivey, #blessed_reviewers, epriestley
    
    Subscribers: avivey, epriestley, Korvin
    
    Maniphest Tasks: T8560
    
    Differential Revision: https://secure.phabricator.com/D13338
Commits on Jun 18, 2015
  1. @joshuaspence

    Use PhutilInvalidStateException

    joshuaspence authored
    Summary: Use `PhutilInvalidStateException` where appropriate.
    
    Test Plan: N/A
    
    Reviewers: epriestley, #blessed_reviewers
    
    Reviewed By: epriestley, #blessed_reviewers
    
    Subscribers: epriestley, Korvin
    
    Differential Revision: https://secure.phabricator.com/D13333
Commits on Jun 15, 2015
  1. @epriestley

    Fix missing property on ArcanistTestResultParser

    epriestley authored
    Fixes T8554 (properly, this time).
    
    Auditors: joshuaspence
  2. @epriestley

    Fix missing property in ArcanistPhpunitTestResultParser

    epriestley authored
    Fixes T8554.
    
    Auditors: joshuaspence
  3. @joshuaspence

    Add some tests for subclasses

    joshuaspence authored
    Summary: Add some tests to ensure that `ArcanistXHPASTLinterRule` subclasses are properly implemented. This should catch issues such as two linter rules having the same `ID` value. See D13272 for a similar change.
    
    Test Plan: `arc unit`
    
    Reviewers: #blessed_reviewers, epriestley
    
    Reviewed By: #blessed_reviewers, epriestley
    
    Subscribers: epriestley, Korvin
    
    Differential Revision: https://secure.phabricator.com/D13274
  4. @joshuaspence

    Extend from Phobject

    joshuaspence authored
    Summary: All base classes should extend from `Phobject` or some other classes. See D13275 for some explanation.
    
    Test Plan: `arc unit`
    
    Reviewers: #blessed_reviewers, epriestley
    
    Reviewed By: #blessed_reviewers, epriestley
    
    Subscribers: epriestley, Korvin
    
    Differential Revision: https://secure.phabricator.com/D13281
  5. @joshuaspence

    Add a linter rule for extending Phobject

    joshuaspence authored
    Summary: If I understand correctly, all classes should extend from `Phobject`?
    
    Test Plan: This needs some further work
    
    Reviewers: #blessed_reviewers, epriestley
    
    Reviewed By: #blessed_reviewers, epriestley
    
    Subscribers: epriestley, Korvin
    
    Differential Revision: https://secure.phabricator.com/D13275
Commits on Jun 14, 2015
  1. @joshuaspence

    Remove "@stable" annotation

    joshuaspence authored
    Summary: I don't believe that `@stable` is useful anymore?
    
    Test Plan: N/A
    
    Reviewers: epriestley, #blessed_reviewers
    
    Reviewed By: epriestley, #blessed_reviewers
    
    Subscribers: epriestley, Korvin
    
    Differential Revision: https://secure.phabricator.com/D13285
  2. @joshuaspence

    Further improvements to test discovery

    joshuaspence authored
    Summary: I found another issue with T8042... it seems that tests from the root directory (i.e. `src/__tests__` are not being discovered properly). The handling of this case (`$library_path` being the library root directory) can probably be improved, and I am open to suggestions. Depends on D13202.
    
    Test Plan: Added to existing test cases.
    
    Reviewers: #blessed_reviewers, epriestley
    
    Reviewed By: #blessed_reviewers, epriestley
    
    Subscribers: Korvin, epriestley
    
    Differential Revision: https://secure.phabricator.com/D13185
Commits on Jun 8, 2015
  1. @joshuaspence

    Mark some strings for translation

    joshuaspence authored
    Summary: Add some more `pht`izations.
    
    Test Plan: Eyeball it.
    
    Reviewers: #blessed_reviewers, epriestley
    
    Reviewed By: #blessed_reviewers, epriestley
    
    Subscribers: Korvin, epriestley
    
    Differential Revision: https://secure.phabricator.com/D13198
  2. @joshuaspence

    Minor documentation improvements

    joshuaspence authored
    Summary: Minor tidying of documentation and adding some groups.
    
    Test Plan: Eyeball it.
    
    Reviewers: epriestley, #blessed_reviewers
    
    Reviewed By: epriestley, #blessed_reviewers
    
    Subscribers: Korvin, epriestley
    
    Differential Revision: https://secure.phabricator.com/D13135
Commits on Jun 3, 2015
  1. @joshuaspence

    Improve test discovery with PhutilUnitTestEngine

    joshuaspence authored
    Summary:
    Ref T8042. Tests were not being discovered in two different scenarios:
    
      # Files modified at the same level as the library root directory.
      # "Normal" directories like `src/unit/engine`.
    
    This regression was caused by D12689.
    
    Test Plan: Added unit tests.
    
    Reviewers: #blessed_reviewers, epriestley
    
    Reviewed By: #blessed_reviewers, epriestley
    
    Subscribers: Korvin, epriestley
    
    Maniphest Tasks: T8042
    
    Differential Revision: https://secure.phabricator.com/D13126
Commits on Jun 2, 2015
  1. @joshuaspence

    Linter fixes

    joshuaspence authored
    Summary: Apply various minor linter fixes.
    
    Test Plan: `arc lint`
    
    Reviewers: #blessed_reviewers, epriestley
    
    Reviewed By: #blessed_reviewers, epriestley
    
    Subscribers: aurelijus, Korvin, epriestley
    
    Differential Revision: https://secure.phabricator.com/D13106
Commits on Jun 1, 2015
  1. @epriestley

    Fix a translation ("to ignore these changes...")

    epriestley authored
    Summary: Fixes T8374.
    
    Test Plan:
    ```
    $ arc diff
    You have untracked files in this working copy.
    
      Working copy: /Users/epriestley/dev/core/lib/arcanist/
    
      Untracked changes in working copy:
      (To ignore this change, add it to ".git/info/exclude".)
        derp
    
        Ignore this untracked file and continue? [y/N] ^C
    ```
    
    Reviewers: btrahan, joshuaspence, chad
    
    Reviewed By: chad
    
    Subscribers: epriestley
    
    Maniphest Tasks: T8374
    
    Differential Revision: https://secure.phabricator.com/D13096
  2. @joshuaspence

    Split the `ArcanistXHPASTLinter` into modular rules

    joshuaspence authored
    Summary:
    The `ArcanistXHPASTLinter` class is becoming quite bloated. This diff separates the class into one-class-per-rule, which makes everything much more modular. One downside to this decoupling is that code reuse between linter rules is much more difficult, although this only affects a very small number of linter rules.
    
    There is still some further work that could be done here, but I defer this until a later diff:
    
      - Rewrite `ArcanistPhutilXHPASTLinter` using `ArcanistXHPASTLinterRule`.
      - Change the unit tests so that they are truly only testing a single linter rule.
      - Maybe improve the way in which linter configuration options are handled.
      - Make it easier to keep track of the linter rule IDs (see T6859).
    
    Test Plan: `arc unit`
    
    Reviewers: epriestley, #blessed_reviewers
    
    Reviewed By: epriestley, #blessed_reviewers
    
    Subscribers: johnny-bit, epriestley, Korvin
    
    Differential Revision: https://secure.phabricator.com/D10541
Commits on May 28, 2015
  1. @epriestley

    Always return an array from ArcanistWorkflow->loadProjectRepository()

    epriestley authored
    Summary: Fixes T8344. Prior to D12971, this always returned `array()`. It may now sometimes return `null`. Switch the behavior to be more similar to the old behavior.
    
    Test Plan: Inspection.
    
    Reviewers: btrahan, joshuaspence
    
    Reviewed By: joshuaspence
    
    Subscribers: epriestley
    
    Maniphest Tasks: T8344
    
    Differential Revision: https://secure.phabricator.com/D13061
  2. @joshuaspence

    Allow PhutilUnitTestEngine to execute tests from a single path

    joshuaspence authored
    Summary: Fixes T8042. Changes the way that `PhutilUnitTestEngine` discovers unit tests. In particular, if you only modify a single test case then there is no reason to run all other test cases within the same directory (which is the current behavior).
    
    Test Plan: Added some unit tests.
    
    Reviewers: epriestley, #blessed_reviewers
    
    Reviewed By: epriestley, #blessed_reviewers
    
    Subscribers: Korvin, epriestley
    
    Maniphest Tasks: T8042
    
    Differential Revision: https://secure.phabricator.com/D12689
Commits on May 27, 2015
  1. @joshuaspence

    Send arcanist error output to STDERR

    joshuaspence authored
    Summary: Currently, arcanist error output is sent to `STDOUT` instead of `STDOUT`. This is annoying because I am running `arc lint --everything --never-apply-patches --output=xml > checkstyle.xml` and the `checkstyle.xml` file is not valid XML.
    
    Test Plan: Forced a linter to throw an exception and ran `arc lint >/dev/null`... saw error output.
    
    Reviewers: #blessed_reviewers, epriestley
    
    Reviewed By: #blessed_reviewers, epriestley
    
    Subscribers: Korvin, epriestley
    
    Differential Revision: https://secure.phabricator.com/D13043
  2. @epriestley

    Push to staging areas when running "arc diff"

    epriestley authored
    Summary: Ref T8238. If a staging area is configured for a repository (see D13019), push a copy of changes to it after creating a diff.
    
    Test Plan: Ran `arc diff` with various options, saw applicable changes get pushed to staging.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: cburroughs, epriestley
    
    Maniphest Tasks: T8238
    
    Differential Revision: https://secure.phabricator.com/D13020
  3. @epriestley

    Provide more documentation for Arcanist upload stuff

    epriestley authored
    Summary: Ref T8259. Make some highly-ambiguous methods like `setData()` more clear.
    
    Test Plan: reading
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T8259
    
    Differential Revision: https://secure.phabricator.com/D13038
  4. @epriestley

    Use file.allocate to upload large files from `arc diff`

    epriestley authored
    Summary: Fixes T8259. Depends on D13016. Use modern logic to support large file uploads.
    
    Test Plan:
      - Diffed with some images, saw them show up in the diff.
      - Diffed with a 12MB binary, saw it upload in chunks.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T8259
    
    Differential Revision: https://secure.phabricator.com/D13017
  5. @epriestley

    Move Conduit file upload logic into a separate class

    epriestley authored
    Summary:
    Ref T8259. Currently, `arc upload` uses new logic but `arc diff` uses older logic internally. This prevents `arc diff` from uploading files larger than 4MB to newer servers.
    
    Split the upload logic apart so the two upload pathways can share it. Callers now build a list of FileDataRefs and hand them to an Uploader for upload.
    
    Test Plan:
    Ran `arc upload` on:
    
      - One file.
      - Several files.
      - Small files.
      - Big files.
      - Directories.
      - Unreadable files.
      - Files full of random data.
      - The same file over and over again.
      - The same big file over and over again.
      - Artificially broke `file.allocate` and redid some of the simple cases (large/chunked aren't expected to work in this case).
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: joshuaspence, epriestley
    
    Maniphest Tasks: T8259
    
    Differential Revision: https://secure.phabricator.com/D13016
Commits on May 26, 2015
  1. @nevogd @epriestley

    Fix invalid parameter in phutil_console_wrap()

    nevogd authored epriestley committed
    Summary:
    Fixes T8314. Change the phutil_console_wrap() call to only have a single
    string parameter.
    
    Test Plan:
    Ran `arc diff` added text into the title, summary and test plan fields,
    but did not specify any reviewers. When prompted to continue, clicked 'No' and
    saw the '(Message saved to commit message.)' string.
    
    Reviewers: #blessed_reviewers, epriestley
    
    Reviewed By: #blessed_reviewers, epriestley
    
    Subscribers: epriestley
    
    Maniphest Tasks: T8314
    
    Differential Revision: https://secure.phabricator.com/D13015
Commits on May 25, 2015
  1. @joshuaspence

    Remove arcanist projects from working copy code

    joshuaspence authored
    Summary: Ref T7604. Remove "arcanist projects" from `ArcanistWorkingCopy` and a few other callsites. Depends on D12999.
    
    Test Plan: Can't really think of how to test this.
    
    Reviewers: #blessed_reviewers, epriestley
    
    Reviewed By: #blessed_reviewers, epriestley
    
    Subscribers: epriestley
    
    Maniphest Tasks: T7604
    
    Differential Revision: https://secure.phabricator.com/D12945
  2. @joshuaspence

    Remove "project.name" from `.arcconfig"

    joshuaspence authored
    Summary: Ref T7604. Remove the `project.name` configuration from `.arcconfig`. Depends on D12999.
    
    Test Plan: N/A
    
    Reviewers: epriestley, #blessed_reviewers
    
    Reviewed By: epriestley, #blessed_reviewers
    
    Subscribers: Korvin, epriestley
    
    Maniphest Tasks: T7604
    
    Differential Revision: https://secure.phabricator.com/D13003
  3. @joshuaspence

    Remove reentrant code

    joshuaspence authored
    Summary: Ref T7604. Remove the `reenter_if_this_is_arcanist_or_libphutil` function. Some discussion in D12945.
    
    Test Plan: Ran `./bin/arc --trace help` and saw things happen as expected.
    
    Reviewers: #blessed_reviewers, epriestley
    
    Reviewed By: #blessed_reviewers, epriestley
    
    Subscribers: Korvin, epriestley
    
    Maniphest Tasks: T7604
    
    Differential Revision: https://secure.phabricator.com/D12999
  4. @joshuaspence

    Remove call to "arcanist.projectinfo" from ArcanistWorkflow

    joshuaspence authored
    Summary: Ref T7604. Remove call to the `arcanist.projectinfo` #conduit endpoint from `ArcanistWorkflow`. Depends on D12992.
    
    Test Plan: Ran `arc which` and verified that repository information was present.
    
    Reviewers: epriestley, #blessed_reviewers
    
    Reviewed By: epriestley, #blessed_reviewers
    
    Subscribers: epriestley
    
    Maniphest Tasks: T7604
    
    Differential Revision: https://secure.phabricator.com/D12971
  5. @joshuaspence

    Fix some format strings

    joshuaspence authored
    Summary: These `pht` calls use `%d` instead of `%s`.
    
    Test Plan: Eyeball it.
    
    Reviewers: epriestley, #blessed_reviewers
    
    Reviewed By: epriestley, #blessed_reviewers
    
    Subscribers: Korvin, epriestley
    
    Differential Revision: https://secure.phabricator.com/D12994
  6. @joshuaspence

    Fix sanity checks for patch workflow

    joshuaspence authored
    Summary:
    Ref T7604. Ref T8307. This was broken in D12962 because I only tested `arc patch --arcbundle`. Furthermore, this particular sanity check doesn't actually do anything now (see T8307).
    
    Test Plan:
    Ran `arc patch --nobranch D12971` successfully.
    
    Auditors: epriestley
  7. @joshuaspence

    Remove call to "arcanist.projectinfo" from `arc export`

    joshuaspence authored
    Summary:
    Ref T7604. Remove a call to `arcanist.projectinfo` from `arc export`. This Conduit call was only used to detect the repository encoding, which we should be able to do locally anyway.
    
    I was considering removing the `$projectName` from `ArcanistBundle` as well, but I'm not convinved that this is a good idea. Specifically, doing so would make it difficult to issue the "This patch is for the 'X' project, but the working copy belongs to the 'Y' project" error. We could potentially use the repository callsign for this purposes, but this isn't portable across installs.
    
    Test Plan: I'm not quite sure how to test this. I suspect that this functionality isn't widely used anyway.
    
    Reviewers: #blessed_reviewers, epriestley
    
    Reviewed By: #blessed_reviewers, epriestley
    
    Subscribers: Korvin, epriestley
    
    Maniphest Tasks: T7604
    
    Differential Revision: https://secure.phabricator.com/D12962
Something went wrong with that request. Please try again.