Page MenuHomePhabricator

csilvers (Craig Silverstein)
User

Projects

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Sunday

  • Clear sailing ahead.

User Details

User Since
May 2 2012, 9:00 PM (755 w, 1 d)
Availability
Available

Recent Activity

Mar 17 2018

csilvers added a comment to T12976: Use a server for pygments?.

If writing, deploying, and maintaining this server doensn't seem like much work to you (and it may reasonably not be), write this server yourself and replace the pygmentize binary on your system with a lightweight client written in a language with a fast startup time that just calls through to the server.

Mar 17 2018, 2:25 AM · Differential

Oct 10 2017

csilvers added a comment to T13007: Integrating `arc lint` and auto-fixers.

Oops, I filed it in the wrong place, my apologies!

Oct 10 2017, 6:39 PM · Lint, Arcanist
csilvers added a comment to T13007: Integrating `arc lint` and auto-fixers.

Doh!, I was looking at the jshint engine before, not the eslint engine. It looks like phab doesn't have built-in support for eslint yet. But this looks promising: maybe the autofixing eslint reports *is* on an error-by-error basis.

Oct 10 2017, 6:04 PM · Lint, Arcanist
csilvers created T13007: Integrating `arc lint` and auto-fixers.
Oct 10 2017, 5:11 PM · Lint, Arcanist

Sep 7 2017

csilvers added a comment to T12976: Use a server for pygments?.

Writing, deploying, and supporting an additional service is an enormous amount of work for us and not a solution I'm interested in pursuing for this problem

Sep 7 2017, 2:52 PM · Differential
csilvers added a comment to T12976: Use a server for pygments?.

(As a side note, the way we'd support this on our ubuntu phabricator machine is to write an upstart script to start the pygments server on startup -- it would also restart the server if it died -- and to run the server under pypy for even more performance happiness.)

Sep 7 2017, 6:32 AM · Differential
csilvers created T12976: Use a server for pygments?.
Sep 7 2017, 6:29 AM · Differential

Jul 17 2017

csilvers created T12925: Warn more loudly when diffusion setups are broken?.
Jul 17 2017, 10:32 PM · Diffusion

Jul 8 2017

csilvers added a comment to T10183: Allow Herald to add a comment to a Maniphest task.

Can you change the syntax to something like @unittest.skip("Reasoning behind this decision...")?

Jul 8 2017, 3:06 AM · Herald, Feature Request
csilvers added a comment to T10183: Allow Herald to add a comment to a Maniphest task.

I know about T9853, but if there's still a chance you might consider merging that upstream, there are a few folks here at Khan Academy who are excited by the feature. Here's one example:

Jul 8 2017, 12:40 AM · Herald, Feature Request

Jun 26 2017

csilvers added a comment to T9898: `git branch --contains X` has cost in the realm of O(commits * branches).

We've run into this same problem, but in the phabricator daemons. They also, apparently, run commands like:

git branch --verbose --no-abbrev --contains <sha> --
Jun 26 2017, 9:13 PM · Restricted Project, Restricted Project, Arcanist

Jan 18 2017

csilvers added a comment to T3917: Avoid or easily handle 'duplicate' audits.

The proposed heuristic (different author and committer) feels pretty shaky, since if you rebase some of your own commits they'll have the same author and committer. I would guess this is somewhat-common in mis-rebases from master?

Jan 18 2017, 9:33 PM · Restricted Project, Audit

Jan 6 2017

csilvers closed T12073: Phabricator does not change status to accepted if you comment when you accept as Resolved.

n/m, I see this was fixed today with https://secure.phabricator.com/rP1f2306999bb599bcedc0c4bfff2f2b3aade58630

Jan 6 2017, 12:50 AM · Differential, Bug Report
csilvers added a comment to T12073: Phabricator does not change status to accepted if you comment when you accept.

download.png (4×1 px, 844 KB)

Jan 6 2017, 12:45 AM · Differential, Bug Report
csilvers added a comment to T12073: Phabricator does not change status to accepted if you comment when you accept.

Jan 6 2017, 12:44 AM · Differential, Bug Report
csilvers created T12073: Phabricator does not change status to accepted if you comment when you accept.
Jan 6 2017, 12:40 AM · Differential, Bug Report

Jan 5 2017

csilvers added a comment to T12070: phd no longer recognizes PHABRICATOR_ENV?.

Am glad to get ahead of it. What's the exact incantation we need to put into our php.ini?

Jan 5 2017, 11:01 PM · Daemons, Bug Report
csilvers added a comment to T12070: phd no longer recognizes PHABRICATOR_ENV?.
ubuntu@phabricator:~$ php -i | grep variables_order
variables_order => GPCS => GPCS
Jan 5 2017, 10:46 PM · Daemons, Bug Report
csilvers created T12070: phd no longer recognizes PHABRICATOR_ENV?.
Jan 5 2017, 10:02 PM · Daemons, Bug Report
csilvers created T12069: `arc diff --reviewers` inserts reviewers twice..
Jan 5 2017, 9:50 PM · Arcanist, Bug Report

Nov 17 2016

csilvers added a comment to T11882: Suggestion for easier port handling when setting up phabricator as a git repo.

(In fact, HostName seems to default to Host, so you could just do

Host phabricator-git.myorg.com
Port 2222

)

Nov 17 2016, 9:57 PM · Feature Request
csilvers reopened T11882: Suggestion for easier port handling when setting up phabricator as a git repo as "Open".

I didn't realize all the issues around munging urls in the UI.

Nov 17 2016, 9:56 PM · Feature Request

Nov 16 2016

csilvers added a comment to T11882: Suggestion for easier port handling when setting up phabricator as a git repo.

Fair enough.

Nov 16 2016, 6:21 PM · Feature Request
csilvers created T11882: Suggestion for easier port handling when setting up phabricator as a git repo.
Nov 16 2016, 5:58 PM · Feature Request

Jul 22 2016

csilvers added a comment to T8115: arcanist plugin system.

arc upgrade: Upgrade everything? This feels sort of bad/dangerous, maybe? But maybe fine? We could prompt?

Jul 22 2016, 4:41 PM · Arcanist

Jul 8 2016

csilvers added a comment to T11301: git tags can tag anything, break daemons.

D16149 has this code:

if ($tag_type == 'tag') {
  $this->log(pht('Skipping, this is a tag of a tag.'));
  continue;
}

Maybe the right thing to do is to check for != 'commit' instead?

Jul 8 2016, 8:50 PM · Bug Report

May 13 2016

csilvers added a comment to T4752: Daemons may not have permission to remove files from local disk storage engine.

Oh, I see I already made that suggestion! I am nothing if not consistent.

May 13 2016, 10:18 PM · Files
csilvers added a comment to T4752: Daemons may not have permission to remove files from local disk storage engine.

In the meantime, we could maybe do the following:

May 13 2016, 10:18 PM · Files

Dec 15 2015

csilvers added a comment to T9993: arc land incorrectly reports I used `--onto`.

w00t w00t, I got something out of this bug report after all!

Dec 15 2015, 6:38 PM · Arcanist, Bug Report
csilvers closed T9993: arc land incorrectly reports I used `--onto` as Invalid.
Dec 15 2015, 6:37 PM · Arcanist, Bug Report
csilvers added a comment to T9993: arc land incorrectly reports I used `--onto`.

I don't have a shell alias, but I do have a wrapper script around arc. Doh! I totally forgot about that. Sorry for the false alarm.

Dec 15 2015, 6:36 PM · Arcanist, Bug Report
csilvers created T9993: arc land incorrectly reports I used `--onto`.
Dec 15 2015, 6:19 PM · Arcanist, Bug Report

Nov 24 2015

csilvers created T9846: Diff on `arc lint` misleading when the replacement has newlines.
Nov 24 2015, 10:30 PM · Restricted Project, Arcanist, Lint

Sep 16 2015

csilvers closed T9426: Can no longer run aliased commands as Resolved.

Nope. I should have tried updating before filing this bug, sorry!

Sep 16 2015, 12:12 AM · Arcanist

Sep 15 2015

csilvers created T9426: Can no longer run aliased commands.
Sep 15 2015, 11:39 PM · Arcanist

Sep 3 2015

csilvers added a comment to T9339: Config Edit "Value" field is confusing.

(I think I just repeated what avivey said, much more crisply, above.)

Sep 3 2015, 6:46 PM · Config
csilvers added a comment to T9339: Config Edit "Value" field is confusing.

(But, and this is a slightly separate issue, it might be nice to have some indication in the 'default' section of which of those values is the current actual value -- maybe just highlight the value, or put a star next to it, or the text 'current working value' or some such. It's not necessary, and I know it will always be the first non-empty value that's marked in that way, but it would remove all ambiguity, especially in cases like the database has the empty string as its value, and a config file has a non-empty string, so the empty string is the default but it may not be clear from the UI.)

Sep 3 2015, 6:46 PM · Config
csilvers added a comment to T9339: Config Edit "Value" field is confusing.

Yeah, I think just hiding the "value" section for non-editable controls would have resolved the issue for me. (In that case, I would have assumed the value was set to the first non-empty value in the "default" section.)

Sep 3 2015, 6:43 PM · Config
csilvers added a comment to T9339: Config Edit "Value" field is confusing.

btw, I think the right fix is to have 'value' be the current value, no matter where it comes from.

Sep 3 2015, 5:47 PM · Config

Aug 31 2015

csilvers added a comment to T9298: Lint/unittest excuses overwrite each other.

I don't know where I'd look for documentation; where the excuse was I guess.

Aug 31 2015, 7:52 PM · Differential
csilvers created T9298: Lint/unittest excuses overwrite each other.
Aug 31 2015, 6:26 PM · Differential

Jun 29 2015

csilvers created T8714: Obscure error when a UnitEngine returns wrong result type.
Jun 29 2015, 11:26 PM · Unit, Arcanist

Apr 5 2015

csilvers added a comment to T7760: cannot add users to projects on the mobile site.

My apologies! I looked at that menu and somehow totally missed it.

Apr 5 2015, 4:05 AM · Projects, Mobile

Apr 4 2015

csilvers created T7760: cannot add users to projects on the mobile site.
Apr 4 2015, 8:07 PM · Projects, Mobile

Mar 27 2015

csilvers added a comment to T1460: Add "Done" link to an inline comment.

This feature is a big hit over here at khan academy!

Mar 27 2015, 8:37 PM · User Delight, Differential

Feb 20 2015

csilvers updated subscribers of T1315: Phacility Launch Status.
Feb 20 2015, 9:22 PM · Phacility

Dec 10 2014

csilvers added a comment to T5553: Allow SSL certificate to be stored in Passphrase.

swisspol: what we do, which could be a workaround for you, is to store the private key in secret, and upload the certificate via /file. Files have access control as well, so you can limit access to the certificate. Then the description in passkey is something like 'Download the certificate at Fxxxx'.

Dec 10 2014, 8:01 PM · Passphrase

Dec 4 2014

csilvers created T6693: Unable to view 'older changes'..
Dec 4 2014, 7:11 PM · Differential

Dec 3 2014

csilvers added a comment to T6680: Tokenizer input field loses focus after tab completion.

Oh! I didn't think of hitting enter (I'm used to tab-completing instead).

Dec 3 2014, 9:13 PM · PHUI
csilvers created T6680: Tokenizer input field loses focus after tab completion.
Dec 3 2014, 12:08 AM · PHUI

Nov 18 2014

csilvers added a comment to T6584: arc diff --verbatim mis-parses for previous review sometimes.

Oops, my mistake -- the usage exception didn't reference Dxxxx at all. Instead, it referenced the differential revision of the change before mine.

Nov 18 2014, 8:18 PM · Arcanist
csilvers created T6584: arc diff --verbatim mis-parses for previous review sometimes.
Nov 18 2014, 8:15 PM · Arcanist

Nov 15 2014

csilvers created T6562: Title/Description querying for Passphrase.
Nov 15 2014, 1:25 AM · Passphrase

Nov 14 2014

csilvers created T6556: Don't jump to a new page for comments when they are on the left.
Nov 14 2014, 5:56 PM · Differential

Nov 12 2014

csilvers created T6540: In diffs, show the vertical "code moved to" bars on the left side as well.
Nov 12 2014, 6:56 PM · Differential

Oct 30 2014

csilvers created T6429: Provide a way to 'import' calendar entries.
Oct 30 2014, 6:19 PM · Calendar

Oct 20 2014

csilvers closed T6356: Mark availability of reviewers as Invalid.

Doh! That's the awsum.

Oct 20 2014, 9:57 PM · Differential
csilvers created T6356: Mark availability of reviewers.
Oct 20 2014, 9:54 PM · Differential

Oct 6 2014

csilvers added a comment to T6255: Bug in the faking of 'production.conf'.

Sorry, I've been having other phabricator fires today. And I changed my config to no longer import production.conf, so it's a bit hard for me to test it -- sorry!

Oct 6 2014, 10:45 PM · Support Impact, Phabricator
csilvers created T6255: Bug in the faking of 'production.conf'.
Oct 6 2014, 5:56 PM · Support Impact, Phabricator

Sep 5 2014

csilvers added a comment to T4752: Daemons may not have permission to remove files from local disk storage engine.

Another possibility -- I'm not saying this is a good idea -- is to use unix groups. That is, have users set up a new "phabricator" unix group, and add the webserver and daemon users to that group. Then use umask to make sure that everything that the webserver and the daemons do are group-writable, and that they chgrp() to the phabricator group when starting up.

Sep 5 2014, 6:48 PM · Files

Aug 26 2014

csilvers added a comment to T5972: Linkify differential emails more.

Ah, I understand; the existing links are the email client auto-linkifying for me.

Aug 26 2014, 5:23 PM · Maniphest
csilvers created T5972: Linkify differential emails more.
Aug 26 2014, 5:14 PM · Maniphest

Aug 4 2014

csilvers added a comment to T5781: Moar functionality for `arc browse`!.
I don't see a reasonable way to build --the-related-review-for that will work with everything for free.
Aug 4 2014, 5:52 PM · Arcanist
csilvers added a comment to T5781: Moar functionality for `arc browse`!.
arc browse --the-related-revision-for <something> is a little more fuzzy and nongeneral. It would be nice if there was a more clear syntax extension path for --the-related-task-for, --the-related-pull-request-for (both of which may match more than one object), and so on.
Aug 4 2014, 5:37 PM · Arcanist
avivey awarded T5781: Moar functionality for `arc browse`! a Evil Spooky Haunted Tree token.
Aug 4 2014, 4:48 PM · Arcanist
csilvers added a comment to T5781: Moar functionality for `arc browse`!.

Are you thinking arc diff + arc diff --browse, or just arc diff --browse which does an arc diff and then loads it in the browser?

Aug 4 2014, 5:44 AM · Arcanist
csilvers created T5781: Moar functionality for `arc browse`!.
Aug 4 2014, 3:25 AM · Arcanist

Apr 15 2014

csilvers added a comment to T4804: Renice daemons to run at lower priority.

According to the ionice man page:

Apr 15 2014, 7:10 PM · Daemons

Mar 19 2014

csilvers added a comment to T4649: `arc diff --verbatim` failing to prefill revision information on update.

Upon further experimentation: it happens even if I don't do 'git amend', but do a regular commit. Basically, if I do 'arc diff --verbatim' on an existing review, it always says 'D' rather than 'D1234'.

Mar 19 2014, 10:36 PM · Differential

Nov 22 2013

csilvers closed T4145: Spacing is totally wonky when viewing reviews on android browser as Wontfix.
Nov 22 2013, 7:31 PM · Differential
csilvers added a comment to T4145: Spacing is totally wonky when viewing reviews on android browser.

OK, understood. Thank you for taking the time to look into it! I'm glad that there's a straightforward workaround.

Nov 22 2013, 7:31 PM · Differential
csilvers added a comment to T4145: Spacing is totally wonky when viewing reviews on android browser.

(If you do install the androvm emulator, the browser is the 'world' icon in the lower right corner at http://androvm.org/wp-uploads/2012/11/step6-359x300.png).

Nov 22 2013, 4:29 AM · Differential
csilvers added a comment to T4145: Spacing is totally wonky when viewing reviews on android browser.

Yeah, if you don't have 'browser' showing up as an installed app for you, there's probably no way of getting it.

Nov 22 2013, 4:26 AM · Differential
csilvers added a comment to T4145: Spacing is totally wonky when viewing reviews on android browser.

The default android browser is called 'Browser', and I believe is installed on default on all android phones (it's the browser used in WebView). My phone has a search bar at the top of every screen; if I type a query into that, it displays search results using the default browser. When I type a url into the search bar, it takes me to that page using the default browser as well. I don't know if your experience would be the same as mine, or different.

Nov 22 2013, 4:19 AM · Differential
csilvers added a comment to T4145: Spacing is totally wonky when viewing reviews on android browser.

(That said, kitkat is replacing the stock android browser with chrome, and the text displays fine in chrome-on-android, so if you wait out the problem it will eventually go away. :-) )

Nov 22 2013, 4:00 AM · Differential
csilvers reopened T4145: Spacing is totally wonky when viewing reviews on android browser as "Open".

I misspoke, my apologies. I'm actually on jellybean. Since kitkat is just now rolling out, and that in a limited way, I think it's pretty accurate to say I'm on the latest version of android. While I don't know for sure, I suspect that everyone who uses phabricator on android has the same user experience that I do. (If not, then it may be a problem with my phone in particular, though I don't know what that problem would be.)

Nov 22 2013, 3:51 AM · Differential
csilvers added a comment to T4145: Spacing is totally wonky when viewing reviews on android browser.

Yeah, I don't actually mind the 2-up view -- I don't know what the 1-up view will look like, so can't say whether I'd prefer to use that or the 2-up view on my mobile device -- it's just the spacing. If magic meta tags could fix it, that would be da bomb!

Nov 22 2013, 2:45 AM · Differential
csilvers added a comment to T4145: Spacing is totally wonky when viewing reviews on android browser.

It just occurred to me the problem may be the font used is not being downloaded or rendered on my phone properly. Not sure if that is my fault or not.

Nov 22 2013, 1:04 AM · Differential
csilvers attached 0 file(s) of 1: Restricted File
Nov 22 2013, 1:03 AM · Differential
csilvers raised the priority of T4145: Spacing is totally wonky when viewing reviews on android browser from to Needs Triage.
Nov 22 2013, 1:02 AM · Differential

Oct 28 2013

csilvers added a comment to T4031: Better handle duplicated audits/maniphest tasks.

Ah, I think that would solve it indeed!

Oct 28 2013, 5:31 PM · Phabricator

Oct 27 2013

csilvers raised the priority of T4031: Better handle duplicated audits/maniphest tasks from to Needs Triage.
Oct 27 2013, 7:09 PM · Phabricator

Oct 4 2013

csilvers added a project to T3917: Avoid or easily handle 'duplicate' audits: Audit.
Oct 4 2013, 8:16 PM · Restricted Project, Audit