As you might already have guessed, this isn't a bug.

Chrome 56 has merged a fix for this so it should be moot in the not-too-distant future, although I'm not exactly sure what the Chrome release schedule looks like. If this only impacts ghost inlines for a few more weeks it's pretty tempting to just wait for it to resolve itself, but I may be digging around in that code soon so perhaps I'll take a more detailed look.

Is it possible that this issue effects Differential but only for inline-comments that have been ported forward from a previous diff (i.e. the ones that are rendered as slightly faded). I see this issue on this instance with this revision: https://secure.phabricator.com/D15675, none of the anchor links in the timeline work for me when I use Chrome 55, but I don't see the same issue on reviews where comments are on the most-recent diff (not faded).

I'm going to switch this view to show overall commit audit status, not weird semi-viewer audit status. At one point it wasn't clear how heavily we were going to try to skew views to account for viewer status, but bucketing changes seem to have reduced the need for this and I think extending the Differential model to Audit now makes sense. See also T9430 for some context.

I'm going to merge this into T7076, which discusses revisions instead of commits/audits, but the resolution for both is likely the same and that task has a more complete discussion of context.

I always thought the Hashicorp projects seemed to cope nicely with a lot of patches and contributors. Projects like Packer and Terraform each process boat load of pull requests.

@BYK Definitely sympathetic here, and we are actively looking to add bodies to the upstream.

I don't think the task detail trivializes this. I expect PHP7 issues will take very little time to resolve, perhaps 2-3 hours? Some installs already run PHP7 with varying degrees of success, which implies there are likely few unknowns lurking in the shadows. I believe this is not a large or complex technical task; it is purely a prioritization issue.

I wasn't trying to create a whole new discussion. I respect your priorities and I also understand the engineers vs. money problem. The only major problem I see with the contribution guidelines is this: to be able to contribute more or affect change, you need clout and to get clout you need to contribute and for those contributions to go into the code or even get some feedback we need time from you guys.

Yep and just as sort of an addition to my previous comment and why I said that: I feel like the current Task Detail trivializes what it probably really means to support PHP 7.

Well, priority and planning documents apply to all 2,600 open tasks, here. But I get what you're saying.

I believe the idea of contribution and input changes are entirely up to the project and what they can support, that said it seems entirely off-topic for this task (imho) and the only thing I would suggest is updating the task detail at the top indicating the statements you've (@epriestley and @chad) made about prioritization and planning so there isn't a comment chain every month about this in this ticket :)

I'll also note that this discussion is moot, anyway: this is not blocked on development resources, and is purely a prioritization issue. If we had 10 full time engineers, this still wouldn't be what any of them would be working on.

We were more open to contributions in the past. The high bar contributions now face is a direct response to the enormous maintenance burden that those changes saddled us with. This is explained in Contributing Code and elsewhere.

I have to agree with @BYK, there's been a lot of interest in getting PHP7 compatibility added. Multiple individuals have offered to work on getting 7.1's async working in Phabricator going back to pre-7.1 release. Phabricator contributors have consistently been absent from the discussion or just shut it down. It's stifling progress and causing interested developers to take their talents to other projects where they can actually get things done.

Honestly, I'd have to disagree. I think us maintaining the roadmap means Phabricator grows faster. Taking detours on external patches daily limits our ability to execute on our own goals, and puts us behind on larger features. I do want to hire a developer or two so accepting patches isn't roadblocked as much, but that means prioritizing $$$ and Phacility first.

@chad - You know how can you get more developers? Being more open to contributions :)

I believe this is probably obsoleted by D17192, or will be obsoleted by T2393.

After D17192, all commit states can be queried explicitly:

After D17192, audits bucket into a dashboard like Differential. Feedback from the Differential bucketing changes has generally been positive, so I think this is probably generally a good direction. It also improves consistency between the applications, if nothing else.

I've marked D17192 as resolving this because viewer() now means "viewer, their projects, and their packages", like Differential. exact(<user>) can be used for exactly a specific user.

I would recommend those users see Prioritization and have their business consider paying to have the feature built if it's a limitation they are faced with. We have roughly 2,600 open tasks here and 1.5 developers, Planning has more information on understanding timelines and priorities.

In T11270#207025, @chad wrote:

I can't think of a reason to prioritize this. Specifically, it seems like since Phabricator for most (all?) companies is a business critical piece of software, you'd always choose to run it on the most reliable / stable version of PHP. Is there a reason we should consider that not to be the case?

See Planning if you have questions about how we prioritize tasks.

I'm just going to merge this into T9640, it isn't meaningfully different from an implementation perspective and is the major compatibility issue.

I haven't explain correctly, what I mean is that this task is "Needs Triage" now but you can know how many work you will need to do (more or less) then you can set its priority (low, hight, whatever).

We could resolve this by implementing "Responsible Users", like Differential.

I'm going to merge this into T9430 since they have the same upstream remedy. This query has largely been modern for some time, but I want to sort out T9430 before providing diffusion.commit.search.

Ah, cool. I'll throw something hacky over this for now if I have a chance, then.

I started redesigning the main header last night, don't expect it to land before stable cut this week. I'll probably land it all at once. (Header Redesign + Favorities Menu).

I think the item didn't exist at all before, and T11114 created it as a side effect. All the installs I have immediate access to are up to date, but, e.g., neither the KDE nor WMF installs have "Create Diff" or "Create Revision" in the menu even though I have access to Differential on both.

I can't think of a reason to prioritize this. Specifically, it seems like since Phabricator for most (all?) companies is a business critical piece of software, you'd always choose to run it on the most reliable / stable version of PHP. Is there a reason we should consider that not to be the case?

Awesome! This is what I wanted!

Now that PHP 7.1 is released you could think what will need to be changed and give a priority to this task.

For what it's worth, we tried disabling the sprint-plugin but we still get the same result.

Before the menu item seem to be called Create Diff and it worked. Right now it's called Create Revision and clicking on it shows an error:

Closing for lack of feedback. You can make custom dashboard panels for whatever query / state you want to track.

Actually, I think this is a separate bug, moving to MenuItem, it's still broken there as well. Probably related to T11114

Looking forward to support PHP7

shouldAppearInCommitMessage() is marked as obsolete, but still seems necessary for me to implement a CustomField that behaves as follows:

Yep @epriestley this one looks fixed.

This is 100 years old and I'm no longer sure what it's describing or how to reproduce it. Feel free to file a new issue if this is still a problem. Note that Audit is churning a lot right now (see T10978) so the behavior may change in the near term. This may also be a "viewer audit status" vs "commit overall audit status" issue in some sense (T9482 / T6024).

We no longer offer this option at HEAD. I'm moving us away from the self raise + self accept workflow to align better with Differential and because the state is unintuitive.

I believe D17183 fixes this. If you're still seeing issues after upgrading past that change, file a new issue with modern reproduction instructions.

D17183 fixes half of this: previews are now sorted in creation order, from oldest (top) to newest (bottom). This is the same order used in Differential. We use this order for previews because it makes it easier to review recently added comments while working through a review/audit.

I'm just going to presume this is resolved, in some sense, by D17181, because you can now remove auditors regardless of other outcomes.

Great, thanks for the report! This should now be fixed in HEAD of master, and promote to stable within about 72 hours.

I tested the patch on my install and it restores the old behaviour!

Unfortunately i'm not quite sure from exactly which version we've upgraded from. But you're probably right and it was a version before Aug 17.
I regularly read the release logs and in the ones i checked before upgrading (which was about 2-3 months worth of em) the latest Modernisation seemed the only thing touching something related to the problem.
D16413 probably didn't make it into any changelog or was just such a minor note that i didn't see it.

I believe this behavior accidentally changed in D16413, on Aug 17 2016. That change caused us to use the commit's project tags instead of the repository's project tags.