@devd, I want to make sure I understand this correctly -- the task seems to be about making one specific use case of lint easier (not linting in general) but is vague about what that use case is.

My understanding is roughly:

• Several teams (like Security) want to enforce lint rules across the board
• These teams also have Herald rules that add them as blocking reviews (see https://secure.phabricator.com/T7617#102687)
• You think it's a pain to keep the lint rules and Herald rules in sync
• You believe that, with improvements to the lint process, you could ditch the Herald rules and do everything with lint rules

Is that correct? I'm skeptical that my understanding of your task is correct -- in particular, without either Blocking Reviewers or Audits there's no way for Security team to ensure that people are obeying the lint rules, and you need Herald to trigger Blocking Reviewers or Auditing.

Can you specifically describe what's painful about lint?

I don't think "using grep for linters" is anything inherent to Phabricator -- you're given the lines that changed and can do anything you want. If your linter uses grep, that's your own fault and you should re-write it to be smarter :). Perhaps you want AST parsing, and think that Phabricator should provide AST parsers for many common languages?

What do you mean lint doesn't work "reliably"? Is this referring to the fact that users can skip lint with --nolint, that lint is always skipped during arc land (T5064), or something else?

Overall, I think that this task is very vague and therefore identifies 0 concrete problems, but if we got specific about the problems you're encountering, we could probably file at least a couple of specific tasks and start working on those.

Sorry, I didn't create this task so I have no idea about whats happening here much. The actual concrete tasks were discussed in the one on one with Phacility.

A lint/herald rule process has three components: check and warn someone before they upload a diff that they are doing something not cool, explain options, block commits unless reviewed (in case policy so requires). Right now, this requires writing a linter, with explanations. If the linter requires an executable, you have to make sure it runs on all the different platforms that developers might be using. After that, the linter logic needs to go into Herald: linters can be imperative and can call external programs but Herald is declarative and only supports regexes.

Then, once you are CC'ed on a diff as a (possibly blocking) reviewer, it is impossible to say "ok .. don't tell me again about this diff nor show it in my queue unless a new upload fires another rule". You should come and see my queue to appreciate how useless it is.

The concrete task (IIRC) identified was that I should be able to write a php class that runs on the server and can call arbitrary external programs on a diff before it is uploaded. And warn the developer on their client side "hey! this *will* block. save yourself from grief and do this other thing pointed out in the documentation". And this can then be one centralized place to run this instead of lint in client side and herald rules at the server side. Further, to reduce confusion, this class should be able to specify a message to show in the differential revision UI as to why the block was added. That seems like a pretty clear task to me.

Then, once it is uploaded, it shouldn't show up in the queue for the blocking reviewers as "blocking others" until someone else has accepted and it is ready to ship save for only the blocking reviewer. Finally, if a blocking reviewer accepts a diff, it should stop showing up in a queue but also shouldn't be considered an actual review that greenlits landing since you want another developer who is reading the whole diff to review the diff too. Right now, the security team cannot give a list of diffs that are waiting only on security unblocking (otherwise all other reviews have passed) sorted by age of the diffs.

Now I haven't figured out concrete tasks for this but it is unreasonable to say that any one who is using the product should come up with concrete tasks. I can only point out pain points and what exactly is the fix should be driven by what matches the phabricator philosophy, architecture, and priorities.

This might be sort of a copy of T7617.