[PATCH] D110287: [modules] While merging ObjCInterfaceDecl definitions, merge them as decl contexts too.

Tue Oct 19 17:21:17 PDT 2021

vsapsai added a comment.

In D110287#3074082 <https://reviews.llvm.org/D110287#3074082>, @dblaikie wrote:

> In D110287#3073804 <https://reviews.llvm.org/D110287#3073804>, @vsapsai wrote:
>
>> Pre-merge checks are passing now after the rebase. I believe it is a straightforward change and we are merging decl contexts for other Decls already, so merging them for `ObjCInterfaceDecl` makes sense. If there are no objections, I plan to land the change on Friday, October 22. If any issues come up later, post-commit reviews are welcome as always.
>
> Generally this sort of "if no one says anything I'll commit at this time" thing is to be avoided: The idea is that once something's been sent for review/the author has requested a second opinion, we want to avoid people committing that code without review only due to lack of feedback. Please reach out to reviewers to get a second set of eyes on this before committing.

What would be a better way to deal with changes in areas where I'm more experienced in (Obj-C decl merging on AST deserialization) and still want to run pre-merge checks on other platforms? I got unofficial sign-offs internally but people defer to me because I've worked more in this area.

I believe this change is pretty trivial, while for D110452 <https://reviews.llvm.org/D110452> I want another opinion. But one-line change in D110453 <https://reviews.llvm.org/D110453> is trivial too and I don't think that waiting for reviews for half a year would improve its quality substantially.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110287