[libcxx-commits] [PATCH] D114896: [runtimes][CI] Add a 10 minutes individual test time out

[David Blaikie via Phabricator via libcxx-commits]

dblaikie added a comment.

In D114896#3454476 <https://reviews.llvm.org/D114896#3454476>, @philnik wrote:

> In D114896#3454353 <https://reviews.llvm.org/D114896#3454353>, @dblaikie wrote:
>
>> Generally if something's sent for review it shouldn't be committed until it's approved. If you're looking to run the CI without review, you can use arc's --draft argument to produce a phab 'review' that isn't sent for review, this makes it clear whether something's intended for human review or not.
>
> I think I disagree with you here. When people submit things for review even though they plan to just land it when CI passed it is a lot easier to see what people are up to and what changes (now matter how big) are made to the code base. libc++ is relatively small, so the changes easily affect other people's patches.

The issue is that if someone sends something for review, then commits it without approval - it's possible they felt it needed review, but then committed without it because they weren't willing to wait for review or other reasons. Basically - sending it for review is generally a statement of "this needs a second set of eyes" & then committing without approval seems like walking that back.

If folks want to (& they should, for sure) keep an eye on changes in their area - the place/tools for that are the commits list (filters/searches if you need to pare it down a bit, understandably) and Phabricator's Herald rules ( https://secure.phabricator.com/book/phabricator/article/herald/ )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114896