Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39980 )
Change subject: soc/intel/skylake: vr_config: enable PSI3 and PSI4 by default ......................................................................
Patch Set 8:
whoops, looks like I pushed an older patch when I rebased another change, I'll fix that
Ah, alright. Not a problem (maybe -1 was unnecessary)
A -1 is not a blocker. In fact, it's a good way to indicate mild dissatisfaction with a change, as it stands out in a field of green scores.
but -1 is not a thing that should be used just because of a wrong comment IMHO
There are no real guidelines, what -1 means. So do not interpret too much into it, besides another iteration is needed. -1 scores will get removed with the next iteration, so in my opinion it’s a good way to prevent accidental submission of the change-set.
Agreed, and also, the -1 has a description: "I would prefer that you didn't submit this". Beside some very rare 1 out of some 100 changes, "this" refers to a patch set, not the change itself.
Though, as comments need to be explicitly marked as resolved nowadays, -1 are not needed for that anymore.
ACR has very different mechanics, though. Anyone can mark a comment as resolved, even the change author. But not everyone can remove a -1 (with- out also losing all +2s). Many people mark comments as resolved that really aren't (and comment authors are not notified, unless it's a blunt Done or Ack). Unless we can prevent that, I think -1 is still a very valid choice.