Page MenuHomePhabricator

csilvers (Craig Silverstein)
User

Projects

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Wednesday

  • Clear sailing ahead.

User Details

User Since
May 2 2012, 9:00 PM (344 w, 4 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

May 19 2017

csilvers added a comment to D9370: Implementation of PhutilLexer for Python.

Oh, my assumption was that the raw part was important because of the comment ("// included here for raw strings").

Non-raw strings seem to work correctly without changes (see inline about why):

May 19 2017, 12:43 AM
csilvers added a comment to D9370: Implementation of PhutilLexer for Python.

This was my best guess:

maverick.py
print r"Tom \"Maverick\" Cruise"
print r'Tom \'Maverick\' Cruise'

...but neither string highlights specially and the runtime behavior just makes me confused about why Python has this feature:

$ python maverick.py
Tom \"Maverick\" Cruise
Tom \'Maverick\' Cruise

...what? Why?

May 19 2017, 12:29 AM
csilvers added inline comments to D9370: Implementation of PhutilLexer for Python.
May 19 2017, 12:01 AM

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.

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 inline comments to rPa590e0e753e8: Document even more ways to manage `sshd` ports.
Nov 17 2016, 11:13 PM
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

Nov 23 2015

csilvers added a comment to D14536: Run script-and-regex linter on all files.

Sounds like this ended up having no changes? Let me know if you still need input from me for anything.

Nov 23 2015, 6:57 AM

Oct 19 2015

csilvers accepted D14307: Allow Script-and-Regex linter to have optional/empty capturing patterns for char/line.

ship it!!!!111!1!1

Oct 19 2015, 9:32 PM
csilvers added a comment to D13737: Also coerce "char" for lint messages.

I just tried the patch, and it does indeed fix things for us!

Oct 19 2015, 9:28 PM

Oct 17 2015

csilvers added inline comments to D13737: Also coerce "char" for lint messages.
Oct 17 2015, 12:17 AM
csilvers added inline comments to D13737: Also coerce "char" for lint messages.
Oct 17 2015, 12:08 AM

Oct 8 2015

csilvers added a comment to D14247: Prevent exponential explosion of text-mode rendering of deeply nested blockquotes.

I'm not really understanding what's going on here -- I guess this function must be being called once per level of > seen?, though I don't see where any doubling happens -- but thanks for fixing it up!

Oct 8 2015, 9:08 PM

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

Herald updated subscribers of 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 10 2014

csilvers added a comment to D10465: Add an option to make it easier to debug page hangs.

Cool, I'm an install!

Sep 10 2014, 8:38 PM

Sep 6 2014

csilvers accepted D10428: Reduce the cost of loading large numbers of macros.

I'm happy to approve this, in the interests of fixing our install!

Sep 6 2014, 12:27 AM

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 D10143: Allow "arc browse <symbol>" to resolve symbolic commits.

Oh, I think i was just misundersatnding how the php works. I see the unset commands now, and the logic makes more sense to me. But clearly I am not the best person to be reviewing this...

Aug 4 2014, 6:46 PM
csilvers added a comment to D10143: Allow "arc browse <symbol>" to resolve symbolic commits.

I'm not sure I understand when you use plural arguments -- does that mean you can do arc browse D1234 T5678? If so, am I reading the current review right that it will either interpret all arguments as commits, or all arguments as objects, or all arguments as files? I'm probably not.

Aug 4 2014, 6:17 PM
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 accepted D10142: Add a --browse flag to `arc todo`.

I'm not really competent to review, but seems straightforward enough to me, and it matches what you did for arc diff, so assuming that's ok I am convinced this is too. :-)

Aug 4 2014, 5:43 PM
csilvers added a comment to D10140: Allow "arc browse" to browse objects.

sweet!

Aug 4 2014, 5:41 PM
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 23 2014

csilvers added a comment to D8847: Clarify STRICT_ALL_TABLES instructions.

(Turns out it was an apparmor issue. Aaargh!)

Apr 23 2014, 8:08 PM
csilvers added a comment to D8847: Clarify STRICT_ALL_TABLES instructions.

OK, following the instructions works. It looks like the problem was with permissions on my new .cnf file. (They were too generous? I can't find an error message, sigh. I'm rambling.)

Apr 23 2014, 8:02 PM
csilvers added a comment to D8847: Clarify STRICT_ALL_TABLES instructions.

Thanks! FWIW, I haven't gotten this to work yet (though I was editing a different config file than my.cnf, so I don't yet assume that the instructions here are still problematic).

Apr 23 2014, 7:47 PM

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 27 2014

csilvers accepted D8625: Improve instructions for Google auth mess.

This is perfect, thanks much!

Mar 27 2014, 9:27 PM

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