Skip to content
This repository

Apr 18, 2014

  1. Chad Little

    Add Glyphicons Halflings Font and Examples

    Summary: This adds in the Glyphicons Halflings Font/Iconset as an option for PHUIIconView along with a standard set of 10 colors. This will be a replacement for the standard action icon set in upcoming diffs, as well as obviously give us more flexibility, less KB, and less design resource time managing images.
    
    Test Plan: UIExamples, Diviner
    
    Reviewers: btrahan, epriestley
    
    Reviewed By: epriestley
    
    Subscribers: epriestley, Korvin
    
    Differential Revision: https://secure.phabricator.com/D8798
    authored April 17, 2014

Apr 17, 2014

  1. Evan Priestley

    Record build success or failure on buildable objects

    Summary:
    Fixes T4810. When a buildable completes, make an effort to update the corresponding object with a success or failure message. Commits don't support this yet, but revisions do.
    
    {F144614}
    
    Test Plan:
      - Used `bin/harbormaster build` and `bin/harbormaster update` to run a pile of builds.
      - Tried good/bad builds.
      - Sent some normal mail to make sure the mail reentrancy change didn't break stuff.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T4810
    
    Differential Revision: https://secure.phabricator.com/D8803
    authored April 17, 2014
  2. Evan Priestley

    Implement PhabricatorApplicationTransactionInterface in Differential

    Summary:
    Ref T4810. Ultimate goal is to let Harbormaster post a "build passed/failed" transaction. To prepare for that, implement `PhabricatorApplicationTransactionInterface` in Differential.
    
    To allow Harbormaster to take action on //diffs// but have the transactions apply to //revisions//, I added a new method so that objects can redirect transactions to some other object.
    
    Test Plan:
      - Subscribed/unsubscribed/attached/detached from Differential, saw transactions appear properly.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T4810
    
    Differential Revision: https://secure.phabricator.com/D8802
    authored April 17, 2014
  3. Evan Priestley

    Add `activeDiffPHID` to differential.query

    Summary: Ref T4809. This saves us a few round trips to find a Buildable, and generally makes the notion of "active" more explicit (i.e., not just the diff with the largest ID). In the future, we may let you revert to previous diffs, which would make the "largest number" rule not always correct.
    
    Test Plan: Ran `differential.query`, got sensible results.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T4809
    
    Differential Revision: https://secure.phabricator.com/D8800
    authored April 17, 2014
  4. Evan Priestley

    Give Buildables a status, populate it, and return it over Conduit

    Summary:
    Ref T4809. Currently, buildables have a status field but nothing populates it. Populate it:
    
      - When builds change state, update the Buildable state.
      - Use the new Buildable state on the web UI.
      - Return the new Buildable state from Conduit.
    
    To make it easier to debug/test this:
    
      - Provide `bin/harbormaster update Bxxx ...` to force foreground update of a Buildable.
    
    Test Plan:
      - Used `bin/harbormaster update Bxxx --force --trace` to update buildables.
      - Looked at buidlable list, saw statuses reported properly.
      - Used Conduit to read statuses.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T4809
    
    Differential Revision: https://secure.phabricator.com/D8799
    authored April 17, 2014
  5. Evan Priestley

    Drop nonsense buildStatus field from Buildable

    Summary:
    Ref T4809. Buildables currently have buildStatus and buildableStatus. Neither are used, and no one knows why we have two.
    
    I'm going to use buildableStatus shortly, but buildStatus is meaningless; burn it.
    
    Test Plan: `grep`, examined similar get/set calls, created a new buildable, ran storage upgrade.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T4809
    
    Differential Revision: https://secure.phabricator.com/D8796
    authored April 17, 2014
  6. Evan Priestley

    Add "harbormaster.querybuilds" Conduit API

    Summary:
    Ref T4809. This one is more straightforward. A couple of tweaks:
    
      - Remove the WAITING status, since nothing ever sets it and I suspect nothing ever will with the modern way artifacts work (maybe). At a minimum, it's confusing with the new Target status that's also called "WAITING" but means something different.
      - Consolidate 17 copies of these status names into one method.
    
    Test Plan: Ran some queries via Conduit, got reasonable looking results.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T4809
    
    Differential Revision: https://secure.phabricator.com/D8795
    authored April 17, 2014
  7. Evan Priestley

    Add a rough `harbormaster.querybuildables` Conduit API method

    Summary: Ref T4809. I need to sort out some of the "status" stuff we're doing before this is actually useful (there's no sensible "status" value to expose right now) but once that happens `arc` can query this to figure out whether it needs to warn the user about pending/failed builds.
    
    Test Plan: Ran query with various different parameters.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T4809
    
    Differential Revision: https://secure.phabricator.com/D8794
    authored April 17, 2014
  8. Evan Priestley

    Link to Herald transcripts from Herald transactions

    Summary: See IRC. Some users are having difficulty figuring out why Herald is taking some actions. Make it easier to get to the transcript.
    
    Test Plan: {F144622}
    
    Reviewers: btrahan, chad
    
    Reviewed By: chad
    
    Subscribers: dctrwatson, epriestley
    
    Differential Revision: https://secure.phabricator.com/D8804
    authored April 17, 2014
  9. Evan Priestley

    Improve robustnesss of feed text rendering

    Summary:
    Couple of minor cleanup things here:
    
      - Pass handles to ApplicationTransactions when rendering their stories; this happened implicitly before but doesn't now.
      - Add `?text=1` to do ad-hoc rendering of a story in text mode.
      - Make Conduit skip unrenderable stories.
      - Fix/modernize some text in the Commit story.
    
    Test Plan: Rendered text versions of stories via Conduit and `?text=1`.
    
    Reviewers: btrahan, chad
    
    Reviewed By: chad
    
    Subscribers: zeeg, spicyj, epriestley
    
    Differential Revision: https://secure.phabricator.com/D8793
    authored April 17, 2014

Apr 16, 2014

  1. Evan Priestley

    Allow tasks to yield to other tasks

    Summary:
    For Harbormaster tasks which want to poll or wait, this lets them say "try again a little later" without having to sleep and hold a queue slot.
    
    This is basically the same as failing, except that we don't increment the failure counter. Instead, we just set the current lease to the correct length and then exit. The task will be retried after the lease expires.
    
    Test Plan: Using both `bin/harbormaster` and `phd debug taskmaster`, ran a lot of waiting tasks through the queue, faking them to either yield or not yield in a controlled manner. The queue responded as expected, yielding tasks appropraitely and retrying them later.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Differential Revision: https://secure.phabricator.com/D8792
    authored April 16, 2014
  2. Evan Priestley

    Add a "Create build step" transaction to Harbormaster

    Summary:
    Without this, build steps that have no options (like "wait for previous commits") don't actually save, since the transaction array is empty.
    
    This also generally nice and consistent.
    
    Test Plan: Created a new "wait" step, viewed transaction log.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Differential Revision: https://secure.phabricator.com/D8791
    authored April 16, 2014
  3. Evan Priestley

    Allow Harbormaster build targets to wait for messages

    Summary:
    This hooks up all the pieces of the build pipeline so `harbormaster.sendmessage` actually works. Particularly:
    
      - Candidate build steps (i.e., those which interact with external systems) can now "Wait for Message". This pauses them indefinitely when they complete, until something calls `harbormaster.sendmessage`.
      - After processing a target, we check if we should move it to PASSED or WAITING.
      - Before updating a build, we move WAITING targets with pending messages to either PASSED or FAILED.
      - I added an explicit "Building" state, which doesn't affect workflows but communicates more information to human users.
    
    A big part of this is avoiding races. I believe we get the correct behavior no matter which order events occur in:
    
      - We update builds after targets complete and after we receive messages, so we're guaranteed to update once both these conditions are true. This means messages can't be lost (even if they arrive before a build completes).
      - The minor changes to the build engine logic mean that firing additional build updates is always safe, no matter what the current state of the build is.
      - The build itself is protected by a lock in the build engine.
      - The target is not covered by an explicit lock, but for all states only the engine (waiting) //or// the worker (all other states) can interact with it. All of the interactions also move the target state forward to the same destination and have no other side effects.
      - Messages are only consumed inside the engine lock, so they don't need an explicit lock.
    
    Test Plan:
      - Made an HTTP request wait after completion, then ran a pile of builds through it using `bin/harbormaster build` and the web UI.
      - Passed and failed message-awaiting builds with `harbormaster.sendmessage`.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley, zeeg
    
    Differential Revision: https://secure.phabricator.com/D8788
    authored April 16, 2014
  4. Evan Priestley

    Allow Harbormaster HTTP steps to pass credentials

    Summary: Fixes T4590. Use the credentials custom field to allow Harbormaster HTTP requests to include usernames/passwords.
    
    Test Plan: Ran a build plan with credentials, verified they were sent to the remote server.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T4590
    
    Differential Revision: https://secure.phabricator.com/D8786
    authored April 16, 2014
  5. Evan Priestley

    Implement smart waits for rarely updated repositories

    Summary:
    Ref T4605. When figuring out how long to wait to update a repository, factor in when it was last pushed. For rarely updated repositories, wait longer between updates.
    
    (A slightly funky thing about this is that empty repos update every 15 seconds, but that seems OK for the moment.)
    
    Test Plan:
    Ran `bin/phd debug pulllocal` and saw sensible calculations and output:
    
    ```
    ...
    <VERB> PhabricatorRepositoryPullLocalDaemon Last commit to repository "rPOEMS" was 1,239,608 seconds ago; considering a wait of 6,198 seconds before update.
    >>> [79] <query> SELECT * FROM `repository` r   ORDER BY r.id DESC
    <<< [79] <query> 514 us
    >>> [80] <query> SELECT * FROM `repository_statusmessage` WHERE statusType = 'needs-update'
    <<< [80] <query> 406 us
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rINIH" is not due for an update for 8,754 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rDUCK" is not due for an update for 14 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rMTESTX" is not due for an update for 21,598 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rQWER" is not due for an update for 14 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rBT" is not due for an update for 13 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rSVNX" is not due for an update for 21,598 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rINIG" is not due for an update for 13 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rHGTEST" is not due for an update for 21,598 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rBTX" is not due for an update for 14 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rGX" is not due for an update for 13 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rMTX" is currently updating.
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rPOEMS" is not due for an update for 6,198 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rPHU" is currently updating.
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rSVN" is not due for an update for 21,598 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rPHY" is currently updating.
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rGTEST" is not due for an update for 21,598 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rINIS" is not due for an update for 6,894 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rARCLINT" is not due for an update for 21,599 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rLPHX" is not due for an update for 1,979 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rARC" is not due for an update for 1,824 second(s).
    <VERB> PhabricatorRepositoryPullLocalDaemon Repository "rINIHG" is not due for an update for 21,599 second(s).
    ...
    ```
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T4605
    
    Differential Revision: https://secure.phabricator.com/D8782
    authored April 16, 2014
  6. Evan Priestley

    Make PullLocal daemon more flexible and transparent about scheduling

    Summary:
    Ref T4605. Fixes T3466. The major change here is that we now run up to four simultaneous updates. This should ease cases where, e.g., one very slow repository was blocking other repositories. It also tends to increase load; the next diff will introduce smart backoff for cold repositories to ease this.
    
    The rest of this is just a ton of logging so I can IRC debug these things by having users run them in `phd debug pulllocal` mode.
    
    For T3466:
    
      - You now have to hit four simultaneous hangs to completely block the update process.
      - Importing repository updates are killed after 4 hours.
      - Imported repository updates are killed after 15 minutes.
    
    Test Plan:
      - Ran `phd debug pulllocal` and observed sensible logs and behavior.
      - Interrupted daemon from sleeps and processing with `diffusion.looksoon`.
      - Ran with various `--not`, `--no-discovery` flags.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T3466, T4605
    
    Differential Revision: https://secure.phabricator.com/D8785
    authored April 16, 2014
  7. Evan Priestley

    Make discovery slightly cheaper in the common case

    Summary:
    Ref T4605. Before discovering branches, try to prefill the cache in bulk. For repositories with large numbers of branches, this allows us to issue dramatically fewer queries.
    
    (Before D8780, this cache was usually held across discovery events, so being able to fill it cheaply was not as relevant.)
    
    Test Plan: Ran discovery on Git, Mercurial and SVN repositories. Observed fewer queries for Git/Mercurial.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T4605
    
    Differential Revision: https://secure.phabricator.com/D8781
    authored April 16, 2014
  8. Evan Priestley

    Separate repository updates from the pull daemon

    Summary:
    Ref T4605. Currently, the PullLocal daemon is responsible for two relatively distinct things:
    
      - scheduling repository updates; and
      - actually updating repositories.
    
    Move the "actually updating" part into a new `bin/repository update` command, which basically runs the pull, discover, refs and mirror commands. This will let the parent process focus on scheduling in a more understandable way and update multiple repositories at once. It also makes it easier to debug and understand update behavior since the non-scheduling pipeline can be run separately.
    
    Test Plan:
      - Ran `update --trace` on SVN, Mercurial and Git repos.
      - Ran PullLocal daemon for a while without issues.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T4605
    
    Differential Revision: https://secure.phabricator.com/D8780
    authored April 16, 2014
  9. Chad Little

    Remove extra workboard margin on mobile

    Summary: We have too much space on workboards when displayed on mobile devices.
    
    Test Plan: Shrink browser display, note that all workboards align to common gutters.
    
    Reviewers: epriestley, btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley, Korvin
    
    Differential Revision: https://secure.phabricator.com/D8790
    authored April 16, 2014
  10. Chad Little

    Add ability to edit Projects on mobile

    Summary: sets action list to crumbs
    
    Test Plan: shrink browser, see mobile action list, click on it, edit
    
    Reviewers: epriestley, btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley, Korvin
    
    Differential Revision: https://secure.phabricator.com/D8789
    authored April 16, 2014

Apr 15, 2014

  1. Evan Priestley

    Let project prefilling accept PHIDs

    Summary: I recently made this better about accepting project names, but we use it in some cases with PHIDs. Make that work properly again.
    
    Test Plan: Clicked "New Task" from a project page.
    
    Reviewers: chad, btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Differential Revision: https://secure.phabricator.com/D8778
    authored April 15, 2014
  2. Evan Priestley

    Fix a lookup issue in Owners

    Summary:
    Fixes T4477. Sort of winging this but it's probably the right fix?
    
    One error in T4477.
    
    One error via email:
    
    ```
    [2014-04-15 17:44:34] ERROR 8: Undefined index: /some_index/ at [/phab_path/phabricator/src/applications/owners/storage/PhabricatorOwnersPackage.php:213]
      #0 PhabricatorOwnersPackage::findLongestPathsPerPackage(Array of size 3 starting with: { 0 => Array of size 3 starting with: { id => 5 } }, Array of size 8 starting with: { / => Array of size 2 starting with: { /some_index/some_file.py => true } }) called at [/phab_path/phabricator/src/applications/owners/storage/PhabricatorOwnersPackage.php:170]
      #1 PhabricatorOwnersPackage::loadPackagesForPaths(Object PhabricatorRepository, Array of size 2 starting with: { 0 => /some_index/some_file.py }) called at [/phab_path/phabricator/src/applications/owners/storage/PhabricatorOwnersPackage.php:119]
    ...
    ```
    
    Test Plan: Will make @zeeg do it.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley, zeeg
    
    Maniphest Tasks: T4477
    
    Differential Revision: https://secure.phabricator.com/D8779
    authored April 15, 2014
  3. Bob Trahan

    Maniphest - remove "attach file" action

    Summary: Fixes T4655. Basically leaves the display code intact for legacy installs but removes the option from the UI and removes "create" code.
    
    Test Plan:
    tried to attach file and the action was not in the dropdown!
    made a new task and it worked!
    commented on an old task and it worked!
    
    Reviewers: epriestley
    
    Reviewed By: epriestley
    
    Subscribers: epriestley, Korvin
    
    Maniphest Tasks: T4655
    
    Differential Revision: https://secure.phabricator.com/D8777
    authored April 15, 2014
  4. Evan Priestley

    Remove the developer-specific CSRF help in phabricator_form()

    Summary:
    Fixes T4802. For context, see T1921.
    
    Originally (in T1921), a developer ran into an issue where rendering `phabricator_form()` with an absolute URI confusingly dropped CSRF tokens, and it wasn't obvious why. This is a security measure, but at the time it wasn't very clear how all the pieces fit together. To make it more clear, we:
    
      # expanded the exception text in developer mode to include a description of this issue; and
      # added an exception in developer mode when rendering a form like this.
    
    However, (2) causes some undesirable interactions for file downloads. In particular, if:
    
      - developer mode is on; and
      - there's no alternate file domain configured; and
      - you try to download a file...
    
    ...we produce CDN URIs that are fully-qualified, and you get the exception from (2) above.
    
    This is kind of a mess, and producing fully-qualified CDN URIs in all cases is simple and clear and desirable. To resolve this, just revert (2). We still have the clarification from (1) above and this hasn't caused further issues, so I think that's sufficient. This is a rare issue anyway and not particularly serious or error prone (at worst, a bit confusing and annoying, but hopefully easy to understand and resolve after the changes in (1)).
    
    Test Plan: With develper mode and no alternate file domain, downloaded files from Files.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T4802
    
    Differential Revision: https://secure.phabricator.com/D8776
    authored April 15, 2014
  5. Evan Priestley

    Make task queue more robust against long-running tasks

    Summary:
    See discussion in D8773. Three small adjustments which should help prevent this kind of issue:
    
      - When queueing followup tasks, hold them on the worker until we finish the task, then queue them only if the work was successful.
      - Increase the default lease time from 60 seconds to 2 hours. Although most tasks finish in far fewer than 60 seconds, the daemons are generally stable nowadays and these short leases don't serve much of a purpose. I think they also date from an era where lease expiry and failure were less clearly distinguished.
      - Increase the default wait-after-failure from 60 seconds to 5 minutes. This largely dates from the MetaMTA era, where Facebook ran services with high failure rates and it was appropriate to repeatedly hammer them until things went through. In modern infrastructure, such failures are rare.
    
    Test Plan:
      - Verified that tasks queued properly after the main task was updated.
      - Verified that leases default to 7200 seconds.
      - Intentionally failed a task and verified default 300 second wait before retry.
      - Removed all default leases shorter than 7200 seconds (there was only one).
      - Checked all the wait before retry implementations for anything much shorter than 5 minutes (they all seem reasonable).
    
    Reviewers: btrahan, sowedance
    
    Reviewed By: sowedance
    
    Subscribers: epriestley
    
    Differential Revision: https://secure.phabricator.com/D8774
    authored April 15, 2014

Apr 14, 2014

  1. pengstanford

    Give the commitownersparser a little more time

    Summary:
    Recently we see issues with huge commits (branch cuts for www) where people received hundreds of emails for the same commit. By checking all the active and archived tasks related to such commits, I saw the following pattern:
     - The commit itself is marked as importStatus = 15 which means all the processing was actually done;
     - In archived tasks, I see one PhabricatorRepositorySvnCommitMessageParserWorker, one PhabricatorRepositorySvnCommitChangeParserWorker, followed by many PhabricatorRepositoryCommitHeraldWorker, which means that the PhabricatorRepositoryCommitOwnersWorker (who schedule those herald tasks) was never done;
     - PhabricatorRepositoryCommitOwnersWorker is always active (for days) with failureCount = 0;
     - In daemon log I see a lot of lease expire exception for PhabricatorRepositoryCommitOwnersWorker.
    So to me it looks like the following happened:
     - Everything is fine until we schedule the PhabricatorRepositoryCommitOwnersWorker
     - PhabricatorRepositoryCommitOwnersWorker actually successfully finished but its running time exceed 60s. Before it finishes, it scheduled the PhabricatorRepositoryCommitHeraldWorker task
     - When we try to archive it, the lease expiration exception happened. As a result, it stayed active and will be picked up immediately since it is in the head of the queue
     - The two steps above repeat forever until we kill it
    I am not sure why we want to check lease expiration when we are archiving the task. For now I am giving the worker a little more time since parsing half million affected path needs some time..
    
    Test Plan: Patched in our production and it worked.
    
    Reviewers: lifeihuang, JoelB, #blessed_reviewers, epriestley
    
    Reviewed By: #blessed_reviewers, epriestley
    
    Subscribers: epriestley, Korvin
    
    Differential Revision: https://secure.phabricator.com/D8773
    authored April 14, 2014 epriestley committed April 14, 2014
  2. Evan Priestley

    Remove some ad-hoc loading of repositories from Releeph

    Summary: Ref T3551. Since we now require repositories in order to perform policy checks, things that did loads properly don't need to load this data explicitly.
    
    Test Plan: Edited a product, cut a new branch.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T3551
    
    Differential Revision: https://secure.phabricator.com/D8769
    authored April 14, 2014
  3. Evan Priestley

    Remove several "loadArcanistProject()" methods

    Summary:
    Ref T3551. Releeph has old-style `loadX()` methods; get rid of one of them.
    
    Differential has a couple of copies of this too, clean them up.
    
    Test Plan:
      - Viewed various differential revisions (with and without projects).
      - Viewed and edited Releeph products.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T3551
    
    Differential Revision: https://secure.phabricator.com/D8768
    authored April 14, 2014
  4. Evan Priestley

    Unban releeph product name "branch"

    Summary:
    Fixes T3657. We no longer construct ambiguous URIs, so product names are no longer restricted.
    
    Also fix some minor URI construction stuff.
    
    Test Plan: Created a product called "branch".
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T3657
    
    Differential Revision: https://secure.phabricator.com/D8767
    authored April 14, 2014
  5. Evan Priestley

    Remove ReleephProjectController

    Summary:
    Ref T3657. General changes here:
    
      - Removes `ReleephProjectController`, which is the source of T3657.
      - Mostly moves requests from "RQ" as a monogram to "Y" (looks like a merge, mnemonic for "yank"?, we don't have too many characters left). This should be essentially only cosmetic. This reduces ambiguity with "rQ" and "R123", which are current and future repository monograms. This will continue in the next few diffs.
      - Makes requests implement policies correctly.
    
    Test Plan: Created, edited, browsed requests.
    
    Reviewers: chad, btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T3657
    
    Differential Revision: https://secure.phabricator.com/D8766
    authored April 14, 2014
  6. Evan Priestley

    Add DifferentialHunkQuery to start hiding hunk storage details

    Summary:
    Ref T4045. We have a lot of direct queries against the hunk table right now. These are messy, not really policy-aware, and limit our options on T4045.
    
    This query is unusual (it requires changesets, and does not accept IDs). This keeps us from having to load changeset -> diff -> revision in order to do policy checks. We could also fix this with smarter policy checks and caching, but I'd rather not open that can of worms for now. This object is very low level and relatively unusual, and this small deviation from convention seems like the cleanest cut to make to keep this from snowballing.
    
    Test Plan: Used Herald dry runs to verify that the affected rules still output the same data.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T4045
    
    Differential Revision: https://secure.phabricator.com/D8765
    authored April 14, 2014
  7. Evan Priestley

    Simplify Herald logic for loading Differential changes

    Summary: Ref T4045. These three methods are fairly copy-pastey. Provide a more formal DifferentialHunk API for querying various types of line ranges.
    
    Test Plan: Used test console to verify that "added content", "removed content", and "changed content" rules still produce the same data.
    
    Reviewers: btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T4045
    
    Differential Revision: https://secure.phabricator.com/D8764
    authored April 14, 2014

Apr 12, 2014

  1. Evan Priestley

    Move Releeph branch controllers toward a modern/stable state

    Summary:
    Ref T3644. Ref T3657. Ref T3549. Basically:
    
      - Move these controllers to modern query/policy infrastructure.
      - Move them to consistent, ID-based URIs.
      - Rename "Project" to "Product"; "Pick Request" to "Pull Request".
      - Clean up a few UI things here and there.
    
    Test Plan:
      - Created and edited branches.
      - Opened and closed branches.
      - Viewed branch history.
      - Searched within a branch.
      - Browsed to branches from products.
    
    Reviewers: chad, btrahan
    
    Reviewed By: btrahan
    
    Subscribers: epriestley
    
    Maniphest Tasks: T3644, T3549, T3657
    
    Differential Revision: https://secure.phabricator.com/D8646
    authored April 12, 2014

Apr 11, 2014

  1. Bob Trahan

    Differential - fix bug writing affected paths

    Summary: Fixes T4774. With the new code and configuration instructions downplaying the role of arcanist project we weren't writing affected paths at all! I had this issue on my installation - no affected paths were written. We seem to always have the repository now though if we can see it, so not too bad of a fix.
    
    Test Plan: updated a diff and was able to browse in diffusion.
    
    Reviewers: epriestley, bitglue
    
    Reviewed By: epriestley, bitglue
    
    Subscribers: bitglue, epriestley, Korvin
    
    Maniphest Tasks: T4774
    
    Differential Revision: https://secure.phabricator.com/D8757
    authored April 11, 2014
  2. Bob Trahan

    Differential - make diffs you authored + are reviewer for show up in …

    …appropos boxes
    
    Summary: Fixes T2328. Note the audit part is fixed now.
    
    Test Plan: Tried to reproduce the audit issue by raising my own commit as a problem; it showed up before code changes! Made a diff with my self as author and reviewer; it showed up as expected
    
    Reviewers: epriestley
    
    Reviewed By: epriestley
    
    Subscribers: epriestley, Korvin
    
    Maniphest Tasks: T2328
    
    Differential Revision: https://secure.phabricator.com/D8755
    authored April 11, 2014
Something went wrong with that request. Please try again.