[PATCH] D128462: [HLSL] add -I option for dxc mode.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

I understand your frustration with the regression, but let’s try to constructive. We all care about quality and we’re all working hard to do the best we can.

In D128462#3794868 <https://reviews.llvm.org/D128462#3794868>, @kadircet wrote:

> My main complaint here's around **breaking** functionality for existing driver-modes/toolchains, and I am asking either getting OWNERS approvals going forward or being more through with the changes or changing the design overall to make these less complicated.

Regressions happen. There are things we can do to avoid them (code review being one), but the most fool-proof way to avoid regressions is test coverage. Here we had a gap in test coverage of existing functionality. A new change went in, all the tests passed, but an important usage regression occurred. It is unfortunate, but it happened.

Getting owner approval before commit has never been required by our developer policy. In fact, the wording of the policy puts the onus on the owner for post-commit review. In this case we had a fairly innocuous seeming change by a relatively new contributor and I reviewed and approved it because it seemed fairly innocuous. This said, hopefully the recent changes in Clang code ownership will help here too.

This change was tested, and introduces a new test in accordance with our community standards and policy.

I don’t think it is fair to say this change contributed to a complicated design other than that llvm’s libOption is pretty complicated and has lots of difficult to see edge cases, and the tools that share option parsing logic often have disparate and duplicated logic for handing those options which can cause a variety of issues.

In terms of your other complaints around bug handling; I also understand frustration there. Unfortunately, clangd has a separate bug reporting and tracking process from the rest of the LLVM and Clang code it depends on. I’m sure it is frustrating to get bug reports that relate to code you didn’t write or design, but I think that is a reality of the process and complexity of the software that we have today. It is incredibly common in LLVM for bugs to get routed to the wrong area and need to be rerouted. Sometimes this is because users reporting issues don’t understand the architecture of the code base well enough to route to the right place, other times it is because the cultural boundaries of responsibility in LLVM aren’t well codified.

In order to support the incremental implementation process that LLVM has always used we need a way to be able to add driver and language modes incrementally. If there’s a gap in the Tooling testing or architecture we can change to make that better, let’s please discuss that, rather than trying to apply additional hurdles to a review process because a regression was introduced.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128462/new/

https://reviews.llvm.org/D128462