Ah ok. So it was only removed from their representation of a pull request. It’s not ideal as you can see either the comments or the file but not both at the same time but at least both are still available.
I meant single commit as in "whole patch series merged into one before push".
This is usually a bad idea, unless the series is super-destructive and
the reviewers suggest a squash to minimise the damage.
One commit per feature / step is the best way, IMO, so I think we're
in agreement.
--renato
> > As I understand it, the GH model is to amend a new commit to your PR
> > which addresses the review comments. The "problem" is that "we" are used
> > to the force push model in which each commit is always as "clean and self
> > contained" as possible (this is not only because of Phab I'd argue).
>
> With Git, *adding* commits to a pull request that address the comments
> is good practice.>> This does not change the the original commits, and in the web UI,
> doesn't disturb the comments.
>
> New comments can be added, and fixups can be approved with a comment:
> "commit X is good, wait for the series to be approved to push".
>
> Better still, it's very easy for anyone (with git) to checkout,
> cherry-pick, rebase and try things locally during code review.Arguably, having clean self contained commits is for this use case even
better. While you can cherry-pick N commits after N rounds of review it
is easier and more convenient to have a single one.
Agreed.
> In the end, when the series is approved, there are a number of options
> before pushing:
> * You can merge as is (works if the author had good commit messages
> for all fixups)
> * You can squash-and-merge, which will add all commit messages to the
> one patch (merger can edit)
> * You can request the author to squash the fixups on the original commitsI can see that a suitable model can be found with the last two, or a
combination of the three.
I find it much more important to have a clean history and would
therefore frown upon the first option; keeping the fixup commits
around is almost never a good idea.
One of the most important goals of history management should be to
have a useful `git bisect`. This requires that every individual commit
leaves the codebase in a "clean" state. (Ideally we'd have bots that
check each individual commit, but that's a resourcing problem.)
> I prefer the last one. It's more work, but it lands "as the author
> intended" because it gives them more time to rework local history.Agreed.
Agreed.
> Single commits are harder to find cause of breakages and the commit
> message may lose info if the merger is not the author.I don't understand:
- Single commits per feature should make it easier to find cause of
breakage. What do I misunderstand?> Multiple useless commits (fix typo, whitespace, reordering
> switch-case) can create rebase problems for everyone else.I think most people are in agreement that those should be avoided if
possible, e.g., as part of a review, but are fine if they come alone as
NFC commits.
I don't quite follow. Yes, gratuitous useless changes tend to create
merge and rebase problems and should generally be avoided. Why does it
make a difference whether they're in multiple commits though?
I agree that it's good practice to have such reorderings etc. in
separate, NFC commits. That's actually one of the reasons why I think
a good flow for patch-series-centric review is so important, because
it facilitates this good practice.
Cheers,
Nicolai
It doesn't have to be so complicated. You can do:
git rebase -i $base -x "arc diff @^"
... which works both for the initial posting and for updating reviews.
Maybe you don't like / can't use(???) Arcanist, but it seems there are
alternatives that are being discussed in other threads.
Cheers,
Nicolai
I think you misunderstood.
Our policy [1] is that independent NFC fixups on existing code should
be separate from new changes.
The point here is that fixups *to the patch* should be squashed into
their original commits *before* merging.
Once the patches are in master, and problems have been found, then
adding a fixup to master is totally fine.
We don't want to rewrite history on master, that'd be a nightmare. If
something is botched, we revert, then reapply.
We also don't want to have fixups to things that haven't landed on
master yet, so cleaning up the patches and series before merging is
highly encouraged.
But during the review, rewriting history of the series itself makes it
hard for any review tool to have meaningful representations.
So, it's better to have separate fixups during review, but we really
should squash them into their related commits in the series before
pushing.
--renato
This isn’t really true for any review tool – some are written to handle this well. E.g. Gerrit is designed exactly for this flow.
In order to track the commits, it uses a “logical identity” for a commit (similar to Phabricator’s “Differential Revision:” line), separate from the commit hash. A local commit hook is used to automatically generate a random number (well, it’s a hash of something, but may-as-well be a random number), and appends it to the commit message (e.g. “Change-Id: I0123456789abcdef0123456789abcdef01234567”). This ID subsequently serves as the review permalink and identity. Because it’s in the message, it’s preserved across rebases and amends.
Anyways, given that ID, it’s easy to track the evolution of each commit in your branch separately, and treat them as separately-reviewable changes, which can each be individually ammended in response to review feedback.
Gerrit also makes it easy to upload an entire branch worth of commits – just “git push” your branch to the magic-review-upload “branch” on the server. When you do that, every commit on your local branch either creates a new review or updates the existing review automatically. And all the reviews are linked together, so you can see the entire chain of reviews for the branch. (example, see “Relation Chain” on the right: https://go-review.googlesource.com/c/go/+/216221/2).
I think this is much nicer than Phabricator’s method, because the Change-Id is added to the message before it’s uploaded, rather than having to go back and amend each message after first upload to insert the server-assigned id. And the entity which is uploaded is an actual git commit, which means it can be more easily worked with (e.g. fetched by someone else), similar to what you can do with github pull requests.
…But I’m not sure bringing up yet another tool really helps anything here…sorry. =p
"Doerfert, Johannes" <jdoerfert@anl.gov> writes:
As I understand it, the GH model is to amend a new commit to your PR
which addresses the review comments.
I'm fine with that during the review process as long as the commit
history is cleaned up before final merge.
The "problem" is that "we" are used to the force push model in which
each commit is always as "clean and self contained" as possible (this
is not only because of Phab I'd argue).
The question is if everything is approved and the author does a final
cleanup as alluded to above, does that final cleanup also need to go
through review?
-David
In order to track the commits, it uses a "logical identity" for a commit (similar to Phabricator's "Differential Revision:" line), separate from the commit hash. A local commit hook is used to automatically generate a random number (well, it's a hash of something, but may-as-well be a random number), and appends it to the commit message (e.g. "Change-Id: I0123456789abcdef0123456789abcdef01234567"). This ID subsequently serves as the review permalink and identity. Because it's in the message, it's preserved across rebases and amends.
I find Phab's handling of the differential revision as confusing as
Gerrit's commit IDs. On simple cases, it does help. When the updates
are extensive and conflicting, they both fall apart.
Of course, this is up to personal preferences. I don't want to start
any long thread about that topic, I don't think it's worth it. 
Gerrit also makes it easy to upload an entire branch worth of commits -- just "git push" your branch to the magic-review-upload "branch" on the server. When you do that, every commit on your local branch either creates a new review or updates the existing review automatically. And all the reviews are linked together, so you can see the entire chain of reviews for the branch. (example, see "Relation Chain" on the right: https://go-review.googlesource.com/c/go/+/216221/2).
That is true. In that sense, Gerrit is better than GH. But GH still
allows you to update your branch and push and the review does get
updated, and to me, that's better (personally) than Phab.
...But I'm not sure bringing up yet another tool really helps anything here...sorry. =p
I don't think we should consider Gerrit, but it's good to know what
other tools do, so that we can have a better perspective on the ones
we're actually considering.
The GH PR way may not be the most full featured but it's simple, "good
enough" (again, personal), and very cheap to maintain.
It'll also make trivial for people to fork LLVM on GH and send PRs.
cheers,
--renato
We don't enforce that now, so I see no reason to start doing it.
Phab reviews, once approved, can have last-minute modifications and
direct commits.
Often reviewers will reply "LGTM with this nit addressed", which means
the author is supposed to fix the code, ammend/squash, and push.
A rebase squash of the fixups is slightly more complex if they need to
apply to different patches in the series, but the author is *expected*
to run a final test on the rebased version *before* pushing.
If not, buildbots will break and patches reverted.
An author that keeps doing that will raise their own bar on future
reviews, where reviewers would start asking them to rebase and apply
before approving and merging.
cheers,
--renato
I would really not want to review a Pull Request that both has multiple “real” commits, as well as unsquashed “fixup” commits in it. That makes it quite difficult to see the final state of things for any of the commits. Since Github lets you see the diff for the entire PR, if the entire branch is intended to be squashed before commit, it seems potentially-OK (if ugly) for it to temporarily have multiple commits during review, rather than force-pushing a new commit.
But, if you’re making a pull request which is intended to result in multiple logically-distinct commits on master, that no longer works. in that case, ISTM that the only realistically-reviewable option is to rebase/squash before uploading every time.
Indeed, this works better when the number of commits is small and the
nature of the fixups is trivial.
If the change is significant, then having the old commit laying around
is counter-productive and rebase-squashing is the only sane way.
--renato
Can we be done with this conversation? I muted this thread once, but after 100 messages it comes back in gmail. =P
When it began, we had already had one long thread on Phab vs. github pull requests. We already discussed it at the SF bay area dev meeting. So far as I am aware, change is not imminent, and if you want to discuss which tool is better, I suggest that you do it over private email or Discord.
Otherwise, you are adding noise (113+ messages) to these lists.
> I don't quite follow. Yes, gratuitous useless changes tend to create
> merge and rebase problems and should generally be avoided. Why does it
> make a difference whether they're in multiple commits though?I think you misunderstood.
Our policy [1] is that independent NFC fixups on existing code should
be separate from new changes.The point here is that fixups *to the patch* should be squashed into
their original commits *before* merging.
Okay, this intention wasn't clear to me from what was written. It
looks like we agree, then.
Once the patches are in master, and problems have been found, then
adding a fixup to master is totally fine.We don't want to rewrite history on master, that'd be a nightmare. If
something is botched, we revert, then reapply.We also don't want to have fixups to things that haven't landed on
master yet, so cleaning up the patches and series before merging is
highly encouraged.But during the review, rewriting history of the series itself makes it
hard for any review tool to have meaningful representations.
This isn't quite true, as others already pointed out, and *not*
rewriting history can make reviews harder as well! In fact, I've *just
now* had that happen to me on another GitHub project, where this
sequence of events happened:
1. PR was opened with a series of commits, one of which (call it
commit B for base) is non-trivial and under review separately as a
different PR.
2. Other reviewer makes comments, asks for some refactoring changes.
3. Author makes those changes, adds them as a fixup commit.
4. I can now no longer usefully review the PR, because I only have two
options, both of which are similarly useless:
4a. I look at all changes in the PR, in which case I get a messy
mixture of commit B (which ought to be reviewed separately) and the
rest.
4b. I look at individual commits in the PR, but then I only see a
stale version of the author's work.
The fixup approach *might* work if there is only a single reviewer,
but even then I suspect things can quickly become messy. And in any
case, the default assumption in LLVM should be that anybody can join a
review at any time.
The one tool that actually gets this right is Gerrit, which
understands commit series *and* allows you to diff between versions of
a commit. It's unfortunate that Gerrit is so ugly that most people
won't even look at it (and it does have other weaknesses as well,
admittedly).
Cheers,
Nicolai
Renato Golin <rengolin@gmail.com> writes:
But during the review, rewriting history of the series itself makes it
hard for any review tool to have meaningful representations.So, it's better to have separate fixups during review, but we really
should squash them into their related commits in the series before
pushing.
+1, regardless of which review system we use.
-David
Nicolai Hähnle <nhaehnle@gmail.com> writes:
I can confirm that this is indeed frustrating.
I am only a user of Clang (and a former very minor contributor to GCC) but I was recently sufficiently piqued by a small Clang diagnostic infelicity that I looked into fixing it, and came up with what appears to this neophyte to be a trivial 2-line fix. As a first-time contributor to Clang, I read the instructions for contributing at <http://clang.llvm.org/get_involved.html>:
"Clang is a subproject of the LLVM Project, but has its own mailing lists because the communities have people with different interests. The two clang lists are:
• cfe-commits - This list is for patch submission/discussion.
[snip]"
And at <https://llvm.org/docs/GettingStarted.html#sending-patches> (via <http://clang.llvm.org/hacking.html#patches>: "To contribute changes to Clang see LLVM's Getting Started page"):
"We don’t currently accept github pull requests, so you’ll need to send patches either via emailing to llvm-commits, or, preferably, via Phabricator."
Having a trivial one-off patch to propose, and presented with a choice of creating a Phabricator account at llvm and learning how to use it or simply sending the patch via email -- obviously I chose the latter [1]. It's only been 10 days but there have been no replies and around 2000 other emails on the list since then. Of those ~2000, I noticed three that were not automatically generated -- one of which was a reply to another newbie, so well done Jonas Toth! [2]
Apart from that one instance of a reply, it would appear that 99+% of the messages on cfe-commits these days are automatically generated and hence that approximately zero people are actively monitoring the mailing list. So it would probably be good to update the contributing instructions to reflect reality.
John
[1] http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20200120/302838.html
[2] http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20200127/304742.html
From: cfe-dev <cfe-dev-bounces@lists.llvm.org> On Behalf Of John Marshall
via cfe-dev
Sent: Friday, January 31, 2020 7:04 AM
To: Jonas Devlieghere via cfe-dev <cfe-dev@lists.llvm.org>
Subject: Re: [cfe-dev] [llvm-dev] Phabricator -> GitHub PRs?> I believe that technically sending patches to the mailing list is
> still a valid way to get your code reviewed. Not everyone monitors the
> mailing list actively though so that might turn out to be more
> frustrating than Phabricator.I can confirm that this is indeed frustrating.
I am only a user of Clang (and a former very minor contributor to GCC) but
I was recently sufficiently piqued by a small Clang diagnostic infelicity
that I looked into fixing it, and came up with what appears to this
neophyte to be a trivial 2-line fix. As a first-time contributor to Clang,
I read the instructions for contributing at
<http://clang.llvm.org/get_involved.html>:"Clang is a subproject of the LLVM Project, but has its own mailing
lists because the communities have people with different interests. The
two clang lists are:
• cfe-commits - This list is for patch submission/discussion.
[snip]"And at <https://llvm.org/docs/GettingStarted.html#sending-patches> (via
<http://clang.llvm.org/hacking.html#patches>: "To contribute changes to
Clang see LLVM's Getting Started page"):"We don’t currently accept github pull requests, so you’ll need to
send patches either via emailing to llvm-commits, or, preferably, via
Phabricator."Having a trivial one-off patch to propose, and presented with a choice of
creating a Phabricator account at llvm and learning how to use it or
simply sending the patch via email -- obviously I chose the latter [1].
It's only been 10 days but there have been no replies and around 2000
other emails on the list since then. Of those ~2000, I noticed three that
were not automatically generated -- one of which was a reply to another
newbie, so well done Jonas Toth! [2]Apart from that one instance of a reply, it would appear that 99+% of the
messages on cfe-commits these days are automatically generated and hence
that approximately zero people are actively monitoring the mailing list.
So it would probably be good to update the contributing instructions to
reflect reality.John
I expect 99+% of the messages on cfe-commits are automatically generated,
but that doesn't mean nobody reads the list. I'm not the only one who
finds the Phabricator UI to be appallingly bad or even impenetrable, for
anything more sophisticated than posting comments. (I also have a recipe
for posting new patches, learned through trial and many errors.)
I certainly don't use the web UI for figuring out which patches to read
and/or comment on; I use the mailing list for that. Regretfully I don't
do much with the Clang sub-project.
The protocol for proposed patches is effectively the same for emailed
patches as for Phab patches: directly CC people who would appear to be
appropriate reviewers, and reply with a "ping" every week or so if there
are no responses. This will bump the patch up in the mailing list queue
on the list, and (one hopes) the direct CC will be noticed by people who
don't ordinarily read the list.
HTH,
--paulr
From: cfe-dev <cfe-dev-bounces@lists.llvm.org> On Behalf Of John Marshall
via cfe-dev
Sent: Friday, January 31, 2020 7:04 AM
To: Jonas Devlieghere via cfe-dev <cfe-dev@lists.llvm.org>
Subject: Re: [cfe-dev] [llvm-dev] Phabricator → GitHub PRs?I believe that technically sending patches to the mailing list is
still a valid way to get your code reviewed. Not everyone monitors the
mailing list actively though so that might turn out to be more
frustrating than Phabricator.I can confirm that this is indeed frustrating.
I am only a user of Clang (and a former very minor contributor to GCC) but
I was recently sufficiently piqued by a small Clang diagnostic infelicity
that I looked into fixing it, and came up with what appears to this
neophyte to be a trivial 2-line fix. As a first-time contributor to Clang,
I read the instructions for contributing at
<http://clang.llvm.org/get_involved.html>:“Clang is a subproject of the LLVM Project, but has its own mailing
lists because the communities have people with different interests. The
two clang lists are:
• cfe-commits - This list is for patch submission/discussion.
[snip]”And at <https://llvm.org/docs/GettingStarted.html#sending-patches> (via
<http://clang.llvm.org/hacking.html#patches>: “To contribute changes to
Clang see LLVM’s Getting Started page”):“We don’t currently accept github pull requests, so you’ll need to
send patches either via emailing to llvm-commits, or, preferably, via
Phabricator.”Having a trivial one-off patch to propose, and presented with a choice of
creating a Phabricator account at llvm and learning how to use it or
simply sending the patch via email – obviously I chose the latter [1].
It’s only been 10 days but there have been no replies and around 2000
other emails on the list since then. Of those ~2000, I noticed three that
were not automatically generated – one of which was a reply to another
newbie, so well done Jonas Toth! [2]Apart from that one instance of a reply, it would appear that 99+% of the
messages on cfe-commits these days are automatically generated and hence
that approximately zero people are actively monitoring the mailing list.
So it would probably be good to update the contributing instructions to
reflect reality.John
I expect 99+% of the messages on cfe-commits are automatically generated,
but that doesn’t mean nobody reads the list. I’m not the only one who
finds the Phabricator UI to be appallingly bad or even impenetrable, for
anything more sophisticated than posting comments. (I also have a recipe
for posting new patches, learned through trial and many errors.)
I certainly don’t use the web UI for figuring out which patches to read
and/or comment on; I use the mailing list for that. Regretfully I don’t
do much with the Clang sub-project.The protocol for proposed patches is effectively the same for emailed
patches as for Phab patches: directly CC people who would appear to be
appropriate reviewers, and reply with a “ping” every week or so if there
are no responses. This will bump the patch up in the mailing list queue
on the list, and (one hopes) the direct CC will be noticed by people who
don’t ordinarily read the list.
+1 to all that from me - I don’t use Phab to manage my review queue - I use the mailing list. I do skim through all the commits lists on a weekly (well, I think it’s been a couple of weeks now) basis & try to CC relevant people on reviews if they’re not something I have the time/knowledge to look at, etc.
- Dave
> From: cfe-dev <cfe-dev-bounces@lists.llvm.org
<mailto:cfe-dev-bounces@lists.llvm.org>> On Behalf Of John Marshall
> via cfe-dev
> Sent: Friday, January 31, 2020 7:04 AM
> To: Jonas Devlieghere via cfe-dev <cfe-dev@lists.llvm.org
<mailto:cfe-dev@lists.llvm.org>>
> Subject: Re: [cfe-dev] [llvm-dev] Phabricator -> GitHub PRs?
>
> > I believe that technically sending patches to the mailing list is
> > still a valid way to get your code reviewed. Not everyone
monitors the
> > mailing list actively though so that might turn out to be more
> > frustrating than Phabricator.
>
> I can confirm that this is indeed frustrating.
>
> I am only a user of Clang (and a former very minor contributor
to GCC) but
> I was recently sufficiently piqued by a small Clang diagnostic
infelicity
> that I looked into fixing it, and came up with what appears to this
> neophyte to be a trivial 2-line fix. As a first-time contributor
to Clang,
> I read the instructions for contributing at
> <http://clang.llvm.org/get_involved.html>:
>
> "Clang is a subproject of the LLVM Project, but has its
own mailing
> lists because the communities have people with different
interests. The
> two clang lists are:
> • cfe-commits - This list is for patch submission/discussion.
> [snip]"
>
> And at
<https://llvm.org/docs/GettingStarted.html#sending-patches> (via
> <http://clang.llvm.org/hacking.html#patches>: "To contribute
changes to
> Clang see LLVM's Getting Started page"):
>
> "We don’t currently accept github pull requests, so you’ll
need to
> send patches either via emailing to llvm-commits, or,
preferably, via
> Phabricator."
>
> Having a trivial one-off patch to propose, and presented with a
choice of
> creating a Phabricator account at llvm and learning how to use it or
> simply sending the patch via email -- obviously I chose the
latter [1].
> It's only been 10 days but there have been no replies and around
2000
> other emails on the list since then. Of those ~2000, I noticed
three that
> were not automatically generated -- one of which was a reply to
another
> newbie, so well done Jonas Toth! [2]
>
> Apart from that one instance of a reply, it would appear that
99+% of the
> messages on cfe-commits these days are automatically generated
and hence
> that approximately zero people are actively monitoring the
mailing list.
> So it would probably be good to update the contributing
instructions to
> reflect reality.
>
> JohnI expect 99+% of the messages on cfe-commits are automatically
generated,
but that doesn't mean nobody reads the list. I'm not the only one who
finds the Phabricator UI to be appallingly bad or even
impenetrable, for
anything more sophisticated than posting comments. (I also have a
recipe
for posting new patches, learned through trial and many errors.)
I certainly don't use the web UI for figuring out which patches to
read
and/or comment on; I use the mailing list for that. Regretfully I
don't
do much with the Clang sub-project.The protocol for proposed patches is effectively the same for emailed
patches as for Phab patches: directly CC people who would appear to be
appropriate reviewers, and reply with a "ping" every week or so if
there
are no responses. This will bump the patch up in the mailing list
queue
on the list, and (one hopes) the direct CC will be noticed by
people who
don't ordinarily read the list.+1 to all that from me - I don't use Phab to manage my review queue - I use the mailing list.
Same for me. I use the mailing list, and skim everything. However, I don't have time to reply to everything, so unless it's something which I really must follow very closely (or an email with no one cc'd, and obviously will need certain people cc'd), I'll wait for "ping" emails to see if it's something I can usefully help move along.
-Hal
>
>
>
>
>
> > From: cfe-dev <cfe-dev-bounces@lists.llvm.org
> <mailto:cfe-dev-bounces@lists.llvm.org>> On Behalf Of John Marshall
> > via cfe-dev
> > Sent: Friday, January 31, 2020 7:04 AM
> > To: Jonas Devlieghere via cfe-dev <cfe-dev@lists.llvm.org
> <mailto:cfe-dev@lists.llvm.org>>
> > Subject: Re: [cfe-dev] [llvm-dev] Phabricator -> GitHub PRs?
> >
> > > I believe that technically sending patches to the mailing list is
> > > still a valid way to get your code reviewed. Not everyone
> monitors the
> > > mailing list actively though so that might turn out to be more
> > > frustrating than Phabricator.
> >
> > I can confirm that this is indeed frustrating.
> >
> > I am only a user of Clang (and a former very minor contributor
> to GCC) but
> > I was recently sufficiently piqued by a small Clang diagnostic
> infelicity
> > that I looked into fixing it, and came up with what appears to this
> > neophyte to be a trivial 2-line fix. As a first-time contributor
> to Clang,
> > I read the instructions for contributing at
> > <http://clang.llvm.org/get_involved.html>:
> >
> > "Clang is a subproject of the LLVM Project, but has its
> own mailing
> > lists because the communities have people with different
> interests. The
> > two clang lists are:
> > • cfe-commits - This list is for patch submission/discussion.
> > [snip]"
> >
> > And at
> <https://llvm.org/docs/GettingStarted.html#sending-patches> (via
> > <http://clang.llvm.org/hacking.html#patches>: "To contribute
> changes to
> > Clang see LLVM's Getting Started page"):
> >
> > "We don’t currently accept github pull requests, so you’ll
> need to
> > send patches either via emailing to llvm-commits, or,
> preferably, via
> > Phabricator."
> >
> > Having a trivial one-off patch to propose, and presented with a
> choice of
> > creating a Phabricator account at llvm and learning how to use it or
> > simply sending the patch via email -- obviously I chose the
> latter [1].
> > It's only been 10 days but there have been no replies and around
> 2000
> > other emails on the list since then. Of those ~2000, I noticed
> three that
> > were not automatically generated -- one of which was a reply to
> another
> > newbie, so well done Jonas Toth! [2]
> >
> > Apart from that one instance of a reply, it would appear that
> 99+% of the
> > messages on cfe-commits these days are automatically generated
> and hence
> > that approximately zero people are actively monitoring the
> mailing list.
> > So it would probably be good to update the contributing
> instructions to
> > reflect reality.
> >
> > John
>
> I expect 99+% of the messages on cfe-commits are automatically
> generated,
> but that doesn't mean nobody reads the list. I'm not the only one who
> finds the Phabricator UI to be appallingly bad or even
> impenetrable, for
> anything more sophisticated than posting comments. (I also have a
> recipe
> for posting new patches, learned through trial and many errors.)
> I certainly don't use the web UI for figuring out which patches to
> read
> and/or comment on; I use the mailing list for that. Regretfully I
> don't
> do much with the Clang sub-project.
>
> The protocol for proposed patches is effectively the same for emailed
> patches as for Phab patches: directly CC people who would appear to be
> appropriate reviewers, and reply with a "ping" every week or so if
> there
> are no responses. This will bump the patch up in the mailing list
> queue
> on the list, and (one hopes) the direct CC will be noticed by
> people who
> don't ordinarily read the list.
>
>
> +1 to all that from me - I don't use Phab to manage my review queue - I
> use the mailing list.Same for me. I use the mailing list, and skim everything. However, I don't
have time to reply to everything, so unless it's something which I really
must follow very closely (or an email with no one cc'd, and obviously will
need certain people cc'd), I'll wait for "ping" emails to see if it's
something I can usefully help move along.-Hal
At some point, when I had a little more time, I searched the
(llvm-commits) mailing list for the word "ping" every few days. Nowadays
I mostly look at my Phabricator dashboard (Active Revisions, recently
changed first) which is pretty full partially due to Herald rules. In
addition I scan the (llvm-commits) list, sorted by most recent thread,
which allows me to keep up with what is going on pretty easily.
Cheers,
Johannes