[PATCH] D138942: Support: Make Wait's SecondsToWait be Optional [NFC]

[Alexander Kornienko via Phabricator via llvm-commits]

alexfh accepted this revision.
alexfh added a comment.

LGTM modulo open comments.



================
Comment at: llvm/unittests/Support/ProgramTest.cpp:427
   ASSERT_NO_ERROR(fs::unlockFile(FD1));
-  ProcessInfo WaitResult = llvm::sys::Wait(PI2, 5 /* seconds */, true, &Error);
+  ProcessInfo WaitResult = llvm::sys::Wait(PI2, 5 /* seconds */, &Error);
   ASSERT_TRUE(Error.empty());
----------------
alexfh wrote:
> aganea wrote:
> > Perhaps `/*SecondsToWait*/` here too?
> Or rather `/*SecondsToWait=*/`, since this format is more common and supported by clang-tidy (https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html). Same for other argument comments.
nit: No space between `/*SecondsToWait=*/` and `5`. Or better clang-format all changed lines (e.g. using clang-format-diff - see https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting).


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

https://reviews.llvm.org/D138942