[libc-commits] [libc] [libc][signal] clean up usage of sighandler_t (PR #125745)

Nick Desaulniers via libc-commits libc-commits at lists.llvm.org
Thu Feb 6 14:39:59 PST 2025


================
@@ -275,13 +275,14 @@ add_header_macro(
   signal.h
   DEPENDS
     .llvm-libc-macros.signal_macros
+    .llvm-libc-types.__sighandler_t
----------------
nickdesaulniers wrote:

Ah!  So in gerrit, there's distinct UI to distinguish between patches in a patch series, vs each individual patches having a revision.

Github does not have this distinction (unfortunately), so it's ambiguous when someone has multiple commits whether their intent was to have a distinct patch series to review vs having multiple revisions of one patch.

Perhaps a tell:
As I've been responding to feedback, I've been pushing new commits on top rather than force pushing.  That's how I would distinguish between the two, but there's no rule in github that that's what you have to do, so it remains ambiguous to reviewers still!

In the case of this PR, I plan to squash when merging.  In that case, clicking through the individual commits is akin to clicking through the revisions of one patch in gerrit; instead, just view the combined diff by clicking "Files Changed" rather than "Commits."

FWIW, I was in the thread when llvm discussed successors to phab, personally advocating for gerrit.

https://github.com/llvm/llvm-project/pull/125745


More information about the libc-commits mailing list