Forgot to specify browser: this is Chrome 58.0.3029.110 (64-bit) on OS X and Linux. Also replicates on Safari 10.1.1 (12603.2.4).
- Queries
- All Stories
- Search
- Advanced Search
- Transactions
- Transaction Logs
All Stories
May 30 2017
For pulls (vs mirrors), we switched to ls-remote to detect no-ops in D17497. The primary motivation was to let us make better clustering decisions, but it also "appeared" to improve performance, which suggests that it may not be outlandish to imagine that git ls-remote would use fewer resources here as well.
Thanks for the detailed response! We hadn't considered that people might tamper with mirror remotes, so you make a very good point there that our proposed change might cause a regression. And you're quite right that logging is our problem.
I'm not sure, since I'm not really sure what problem we're solving. It sounds like there are potentially three motivations:
I actually wasn't sure which to pick myself, since it's not as specific as most bug reports, but it seemed more like undesirable behavior of an existing feature, as opposed to a new feature. Would it be more appropriate to consider this as a feature request instead?
Can you walk me through why you consider this to be a bug report rather than a feature request?
Sounds good. Current trajectory is:
If you're making PHUIXButtonView I can come behind and merge in circle stuff. I don't think that needs to be a separate PHP class. I just need to dig out all the button-css stuff and make it more modular so it's not silly with overrides.
what is this even
(I don't have any plans for making JS and PHP render from the same authority -- I don't see any realistic way to accomplish it.)
Oh there is no PHUIXButtonView...
And, uh, SIMPLE is a color. I'm going to change that first, unless you have some other plan for having the yellow and non-yellow buttons present in the mock?
Though I do circle in JS in lightbox, so there would be a second application
You can just make the circle button a square button as well.
Is there a place in 2-5 years where we can write one UI library and work in PHP and JS
Yes I slimmed them in photoshop :(
Thanks! That should be perfect.
Okay. I have to port whatever we're using to PHUIX so you'll have to retouch both PHP and JS, but I can maybe put PHUIX rendering beside PHUI rendering in UIExamples to make that easier.
Just use SIMPLE for now and I'll retouch as needed. It's probably a little over styled in the mock.
Are those buttons rendered by any existing class, or are they novel elements? The circular menus are presumably PHUIIconCircleView but the other buttons look custom (it looks like the spacing is slightly different from the existing "Simple" buttons, maybe)? Should I add a new mode to PHUIButtonView, or is this an existing mode somewhere that I'm just missing, or are they just "Simple" buttons with slightly Photoshoppy rendering?
If you believe this is not an issue with a bad SAPI binary, please file a new bug report including reproduction steps I can follow to reproduce the issue (see Providing Reproduction Steps for guidance).
Possibly also, add a "See All" link to ApplicationSearch
Sounds reasonable to just improve that.
Yeah, I tried hard to not make it progress-like. My approach was mostly, "if I have a dozen comments across 5 files, how could I assume I've read/addresses each piece of feedback" - and that still leads to some sort of progress bar. If people don't click "done" I think it's reasonable to assume they don't need that level of organization. I also suffer from ADHD and have a difficult time if I don't maintain things in a list.
I'm fine with dropping "no progress bars". I believe most requests we've ever received about inlines -- like T8909#224084 -- are fundamentally misguided. That user wanted features to make managing a review with 100 spelling corrections easier. But these requests are common and I don't have any real reason to believe that making Differential hard to misuse is actually helping anyone or putting any real pressure on attacking the root issues.
I think this feature is just inherently incompatible with my hardline "review shouldn't be a progress bar" view, and it would be more confusing to try to obscure the progress aspect of it.
Hey, we're also interested in this by way of T12545 (which was merged in above). A couple of our teams have historically assigned their oncall/tech-debt tasks to a mailing list user, and used the priority-sorted view of those tasks to run their oncall shifts. That workflow no longer works with the reverse-chronological sort.
What about hiding the x/n count until you've marked 1 as done?
I can also just put next/prev back on inlines if you prefer, and we can deal with the design issue if/when we add a context menu for "View History", "View Raw Remarkup", etc. (T11401).
I don't think we need to keep iterating here -- I don't think any version of this will ever be useful to me personally, and I think I'm clearly fighting a losing battle by trying to keep all the "X / Y" progress-bar-style elements out of the UI.
I also have some ideas around a more minimal UI if you'd like to see one more pass.
hmmm, I'm happy to take another pass here. For myself I feel like you, I don't need it - if I keep reviews small - but I think that's not something we can apply across all our users. I've also found I don't need "done" until I do. There reaches a point where I want to make sure I've addressed everything before sending it back for review, and then that list I feel is really useful.
I think it's fine for none of this to work on mobile for now.
(I'm also ok with none of this on mobile)
Do you want a mobile design? I think we can just hide a div on desktop?
Okay, I'm going to implement these behaviors:
Cool, Safari seems fixed again.
Oh! Works great in Chrome.
I deployed D18042 to secure, but it isn't retroactive. You can either:
Specifically, I got this (mail with HTML and text parts containing over-escaped HTML):
On top of this, I got some email from Pholio with odd HTML behavior, but I can't immediately reproduce it and I don't have any guesses about what's causing it so I'll assume I'm just crazy until someone else hits it.
(See also M1476.)
I've got most of a diff for this.
It's still probably Safari though
Probably permissions, let me see if I can reproduce locally. I thought I tested that but maybe didn't get the "replace an image" transaction specifically.
I tried logged out Safari and saw no images
oh, maybe file permissions then
What I'm seeing, in Chrome?
The first two images seem to have been lost, Pholio bug?
Next steps:
This doesn't appear to describe a bug in Phabricator.
I'm just going to nuke this since there's no repro. This workflow has also been rewritten on the experimental branch and the specific error encountered here should now be impossible.
You can leave comments on the left hand side of a 2-up view -- that is, scrolling in EITHER of these areas on 2-up would trigger the problem:
This is kind of time-specific since it has a version number, but I got Aphlict working on my server by navigating to phabricator/support/aphlict/server and running npm install ws@2.X (literal X) as root. This installed version 2.3.1 (im assuming the latest stable 2.X version). Upon restarting aphlict, the notifications work without raising exceptions.
See T12688#223690.
You should be able to use bin/config delete ui.footer-items --database to fix things.
Thanks for the very detailed response.
May 29 2017
Yep, that's a better solution. In the mean time, we just updated all of our .arcconfig files.
I had indeed implemented precisely that herald rule once I realized that these diffs were being created without repository info attached. I also added repository.callsign to /etc/arcconfig to eliminate the problem going forward.
I think we should probably not follow redirects from arc at all.
We could change the behavior to "tap and hold to begin a range + drag".
We've seen a couple of flavors of this from @swisspol, too. Although the symptom there is a little different (not a policy issue, exactly), the root cause is mostly similar. In particular, we saw:
(In this case, our behavior was strictly wrong, completely accidental, and trivial to fix -- but sooner or later we may make some kind of change which isn't very compatible with this older style of integration in a more fundamental way. When we do, a blog post from 2013 suggesting a set of workarounds at the time won't hold back the tides of change.)
The general story on Build/CI is this:
I think I didn't migrate this since there was no direct analog -- my memory's a little fuzzy, but I think the old query had some bugs / weird behaviors / general differences and there's no exact way to represent it after the change (and even if there was, the newer version -- with bucketing -- is probably way better for most users, so it's good to upgrade anyway). I figured it was better to just break it in an obvious way than subtly change it, which could be more confusing, I think.
There isn't enough information here to reproduce the issue as described. Do you have additional steps I can follow and see the issue locally?
I'm not allowed to close this, but it was fixed by T12757