[lldb] [llvm] Modify the localCache API to require an explicit commit on CachedFile… (PR #136121)

via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 25 11:47:33 PDT 2025


dyung wrote:

> Hmm... perhaps I have misunderstood the LLVM contribution process here. I had assumed that once all the review feedback had been addressed, some time had passed to allow reviewers to comment on the modifications resulting from review feedback, and the "merge" button was available, then that was all that was needed. This was my takeaway from reading the documentation at https://www.llvm.org/docs/Contributing.html . But it sounds like you're saying that a separate approval step is necessary? How is that approval obtained? Where is that process documented?

My understanding of the process is you put up a pull request, ask for review/feedback, and then keep iterating until someone approves your PR at which point you can then submit the request. If after some time you haven't heard back, it is customary to "ping" your review to try and get it back on people's radars as many reviewers get a ton of emails and it may get lost in the noise.

The relevant passage from the document you cited I believe is the following:
```
If you have received no comments on your patch for a week, you can request a review by ‘ping’ing the GitHub PR with “Ping” in a comment. The common courtesy ‘ping’ rate is once a week. Please remember that you are asking for valuable time from other professional developers.

After your PR is approved, you can merge it.
```

The main reason I asked is that this patch came to my attention twice, once when it originally caused a lot of bot failures (I was the person who reverted it originally), and also this second time because it is causing problems with our downstream code (that is our problem to fix).

https://github.com/llvm/llvm-project/pull/136121


More information about the llvm-commits mailing list