RFR: 2331: Improve review notes
Pavel Rappo
prappo at openjdk.org
Thu Jul 18 20:23:47 UTC 2024
On Thu, 18 Jul 2024 19:58:58 GMT, Zhao Song <zsong at openjdk.org> wrote:
>> I was just thinking we should make the minimal necessary changes. I admit that parsing text displayed to the end user isn't best practice, but we do it in many places of SKARA.
>
> It's just a suggestion, since you don't like it and I am ok with your pr, I will approve it now.
Usually, making a minimally necessary change is good, and parsing end-user text is bad. In fact, I'd go to great lengths to avoid "stringly typed" code that operates on strings that are also subject to change.
Here, I initially thought I might be violating some encapsulation boundary by bringing a dependency. Maybe I still am. If nothing else, it's not the first class imported from that package:
import org.openjdk.skara.jcheck.JCheckConfiguration;
+import org.openjdk.skara.jcheck.TooFewReviewersIssue;
-------------
PR Review Comment: https://git.openjdk.org/skara/pull/1679#discussion_r1683433452
More information about the skara-dev
mailing list