I like that reasoning a lot better than mine, and simply omitting type appears to produce the correct behavior in every browser, at least for this file. I'll try that instead, and we can revisit this after we write a video transcoder in PHP and can offer files in multiple formats.

My knowledge is centred around broadcast video / IPTV (UDP multicast) rather than HTML5 video, but that seems fairly reasonable. I don't think anything should try and download the file twice unless it is a complete clownshoes implementation that probably has multiple other serious bugs wasting bandwidth. I think the worst case with no type that might be hit here is that the browser could decide to download the entire file on load to figure out the format and duration, rather than starting with byte-range requests, but this is easy to test and unlikely (I'd expect any sane one to always request chunks).

I can imagine that two <source /> tags might, in some bizarre world, cause browsers to download the file twice when you click "Play", if they're super confused about how to process videos, don't notice that the URLs are the same, and don't hit any caching. But that's a pretty minor bad effect, and I didn't immediately see any kind of bad behavior locally.

Thanks! I wouldn't mind parsing that but I'm hesitant to ask installs to install it -- but it's helpful in understanding that I'm not completely crazy here and that there's at least some basis for "video/mp4" being a quasi-legitimate way to label the video file.

mediainfo is the tool for this, but is really overkill for programmatic use.

One easy approach we could take is just:

An install is hitting the "large blame does not degrade gracefully" issue mentioned in that task

See PHI594, where a real user had the same human name as an external user from another project.

From PHI604, for completeness, on the newer behavior of "Hide Blame":

validation for the configuration option

Yeah, I'm looking at here, assuming it's ahead of last release.

Just to make sure we're talking about the same thing, that feedback is true even of the version at HEAD (blue/grey/orange) vs the version that originally shipped (green/grey/red)?

You could maybe leave the element off unless it is a larger change. I think my concern here is seeing it on every revision becomes something extra I have to mentally process, but if the intent is to either help show small changes, or large changes, it could just be visible in those edge cases. In those cases at least, the element provides context as to why it's shown at all vs. all the time?

I think there's possibly at least some argument that this element could serve noble goals:

I feel your instincts are correct, this information isn't useful in a list form and shouldn't be included. You could make the argument to include it in a hovercard, or bring it into the header on the diff page, and I think those are both fine. Outside of that this seems mostly geared towards avoiding code review, a practice I don't think is well aligned with the overall product goals.

I suppose the approach proposed above gets a questionable result here:

After actually looking at the code, it looks like null can already be returned and works, it's just kind of wrong for some objects (like Dashboards) because they really do have a default policy -- it's just hard-coded.

See PHI612 and elsewhere; using "Take action [the first time this rule matches]" isn't always obvious but is usually a better fit than "When [ thing is created ]" and interacts much better with drafts.

A 600MB patch should not take 3 hours to encode. We should reproduce this and fix whatever the problem is (buffer appends or otherwise), or clearly document why we actually can't fix it.

@epriestley, can you confirm that the above patch should disable the draft state? I have applied it to GHC's Phabricator deployment yet we are still seeing Differentials opened in draft state.

I'm going to tentatively put this (the Phriction rule, specifically) into T13130 for this week although I'm not sure how much work getting getDefaultPolicyForObject() to return the root document policy is and maybe that's Unbelievably Difficult. Let me take a quick look at that and you can either run with it or I can pick it up if it's some kind of weird arcana.

@hach-que @epriestley Is there a workaround for Google Cloud SQL? Short of running a DB container?

In T13130#238582, @epriestley wrote:

Offhand, two possible issues with that:

• At least one install has an enormous number of packages (tens of thousands? Hundreds of thousands? Seventy-two billion?) and the minimum cost for a package is much, much smaller than the minimum cost of a Herald rule. If packages actually created Herald rules this install would probably explode.

13,636.

Ah, yeah. No idea why I typed "blind" instead.

There should be a very small number of readers (2-3?) of this field, so it should be possible to blind them safely like this:

One option is to write "Herald Rules for Herald Rules".

Offhand, two possible issues with that:

See PHI251. This requests an "Ignore generated changes." option for Owners.

Historically, an issue with this was my hesitance to add more "Add Owner As Reviewer If Moon is Full" builtin rules to Owners: Herald already allows you to implement arbitrary rules with Owners, and an implementation of Herald where every possible rule is a dropdown option obviously isn't very good.

See PHI580. An install would like write API access to object relationships (depends on, fixes, etc). This is complicated, but perhaps plannable, and we may be able to put a reasonable facade with partial support into the upstream.

See PHI592. A diff with 600MB of videos required more than two hours to turn into an email patch, which exceeded the task lease duration and cascaded into various other kinds of problems.

See PHI285. An install would like a way to monitor changes to Herald rules.

See PHI251. This requests an "Ignore generated changes." option for Owners.