[PATCH] D31028: [ELF] - Detemplate PltSection<ELFT> section.

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 17 09:48:43 PDT 2017


ruiu added a comment.

>> So you submitted a modified version of this patch as r298065? As a common practice, once you sent a patch for review, you are supposed to wait for an LGTM (if you think that a new patch is obvious, you can submit it to let other people do post-commit review, but it needs to be actually obvious and shouldn't be waiting for an LGTM). I appreciate your recent cleanup commits, but you want to slow down a bit here to follow the rule.
> 
> Not modified version of this patch, but a patch that changes nothing and just removes templating, leaving the single method templated. It is obvious trivial change I think. 
>  And I kept this patch on review because think it is better way and can be committed on top.
> 
> Do you think https://reviews.llvm.org/rL298065 really needed pre-commit review ?

No, r298065 is a modified version of this patch. I suggested doing the thing this patch is attempting to to do in a different way, and r298065 is an implementation of the suggestion. In general, if you get a comment to do a thing in a different way, the new patch to do in the new way is a continuation of the original patch. If my code review were slow, I could have understood, but you are actually getting very fast code review from me every day, so no need to haste that much, right? Please allow me to review your patches.


https://reviews.llvm.org/D31028





More information about the llvm-commits mailing list