labath added a comment.

In D76593#1936757 <https://reviews.llvm.org/D76593#1936757>, @anton.kolesov 
wrote:

> In D76593#1936337 <https://reviews.llvm.org/D76593#1936337>, @labath wrote:
>
> > Sounds like a good idea. Could you do the same for `attach_info` (it looks 
> > like it should be possible)? Otherwise, one is left wondering about what's 
> > the difference...
>
>
> I have that as my next patch - it seems like something that should be in a 
> separate commit, but in the same pull request, but I don't know how to 
> properly submit patch series for review at Phabricator - only through 
> "related revisions", I presume?


You can (and should) use the "related revisions" functionality to denote the 
relationship between patches, but the purpose of this relationship is to make 
it easier to review by explaining what depends on what. We don't really have a 
concept of a "pull request" consisting of multiple patches, which get all 
committed as a group. Normally each patch should make sense on its own, even if 
it's not immediately useful (e.g. because it introduces an interface that is 
not yet used by anyone). The "related" patches can then explain/demonstrate how 
it's going to be used, but the preceding patches don't need to wait for a green 
light on the entire group to be committed. If there's some deviation from this 
(e.g. a preparatory refactoring patch makes some code substantially more 
complicated and so it may not be desired if the latter patch is not accepted, 
or there is some uncertainty about the overall direction of the patchset), it 
should be called out explicitly.

But none of this is really relevant for this patch, which is so simple you 
could have done it in one commit (and it could be argued that it *should* be 
done in a single commit, as doing it partway introduces a somewhat inconsistent 
state, which is undesirable).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76593



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to