[PATCH] D153204: RISCVAsmParser: support comments in more places

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 21 08:57:29 PDT 2023


MaskRay added a comment.

In D153204#4437044 <https://reviews.llvm.org/D153204#4437044>, @barannikov88 wrote:

> @MaskRay
> I don't doubt your experience, nor your will to help others, and I certainly didn't mean to offend. My point was that creating and insisting on an alternative review does not encourage newcomers to contribute.
> I think it would be better if we helped Abel to get his review right instead. It would also save everyone's time arguing about which review is better and relieve the reviewers from having to judge.

Sorry, but I'll disagree here. As I mentioned,

> However, I believe there is a cut-off point, and it would be more beneficial to demonstrate how to fix the issue for a few instances to effectively address this problem.

The patch I submitted was aimed at solving the specific issue at hand.

We could spend more energy on suggesting the exact fix, but then that's just like the reviewers write the entire patch. (Actually I sometimes did this for others, but I don't always.)

Also, there is still an opportunity for @abel-bernabeu to get a credit for a commit

> This patch will address 5 out of the remaining 13 use cases. I believe it effectively tackles the most noticeable issues, but it would be beneficial to address the remaining 8 as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153204



More information about the llvm-commits mailing list