Timeline for In code reviews, should the reviewer always present a solution for issues? [closed]
Current License: CC BY-SA 3.0
31 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Dec 17, 2021 at 15:23 | comment | added | nsandersen | @gnat IMO Reviewer is not obliged, but constructive criticism is always best. "garbage" is not overly helpful. | |
| S May 25, 2017 at 18:01 | review | Reopen votes | |||
| Jun 27, 2017 at 12:32 | |||||
| May 25, 2017 at 17:57 | history | closed |
gnat user22815 maple_shaft♦ |
Opinion-based | |
| May 25, 2017 at 16:53 | comment | added | Davor Ždralo | I would say that it's not reviewer's job to propose a solution at all. | |
| May 25, 2017 at 15:43 | answer | added | John Lawrence Aspden | timeline score: 0 | |
| May 24, 2017 at 15:14 | history | protected | gnat | ||
| May 24, 2017 at 6:41 | vote | accept | Frank Puffer | ||
| May 24, 2017 at 0:15 | answer | added | viraptor | timeline score: 3 | |
| May 23, 2017 at 21:23 | answer | added | Eric Hydrick | timeline score: 0 | |
| May 23, 2017 at 17:41 | answer | added | BagOfSpanners | timeline score: 3 | |
| May 23, 2017 at 16:18 | comment | added | user251748 | @gnat One of my most treasured responses was when a very senior programming colleague gave a two-word reply to an email I sent out to many people describing the status of one of my quite successful projects: "Very nice." | |
| May 23, 2017 at 16:00 | answer | added | JeffO | timeline score: 4 | |
| May 23, 2017 at 15:26 | comment | added | Martin Maat | Pinpointing potential problems could be enough. The reviewer looks at the code as a user, maintainer or designer. Providing a different angle view or spotting issues the coder may not have noticed yet can help the coder to improve on his work. And if you spot something you like, it would not hurt to report that too. It should not be a corrective exercise but rather an enlightening one. That is why it is called "peer review". | |
| May 23, 2017 at 15:23 | answer | added | Karl Bielefeldt | timeline score: 13 | |
| May 23, 2017 at 15:15 | answer | added | candied_orange | timeline score: 18 | |
| May 23, 2017 at 15:03 | answer | added | Doc Brown | timeline score: 35 | |
| May 23, 2017 at 14:38 | comment | added | Frank Puffer | @gardenhead: I am aware that inheritance introduces a lot of complexity and should be avoided in most cases. That's why I didn't want to recommend it. By the way, I was reviewing C++ code. | |
| May 23, 2017 at 14:04 | comment | added | gardenhead | "The standard way to resolve this would be to split the class up and use inheritance. ". I sure hope you're not reviewing my code! | |
| May 23, 2017 at 13:08 | history | edited | Frank Puffer | CC BY-SA 3.0 |
added 11 characters in body
|
| May 23, 2017 at 12:48 | comment | added | gnat | no, I'd say that as a reviewer you're not obliged to tell how to improve it. If you can describe what feels wrong in there, do it; if not - just provide general comment. (One of most useful review comments I recall receiving was literally like "this class is all total garbage") | |
| May 23, 2017 at 12:40 | comment | added | Frank Puffer | @gnat: So you'd say that my first example (splitting a class) is too high level for code review? What about my second example (renaming)? | |
| May 23, 2017 at 12:39 | answer | added | guillaume31 | timeline score: 30 | |
| May 23, 2017 at 12:19 | answer | added | Bart van Ingen Schenau | timeline score: 141 | |
| May 23, 2017 at 12:00 | comment | added | gnat | if you don't have enough time to come up with specific solution during code review then it's definitely too complicated (in the context of code review). And the top answer over there appears to be spot on (second and third top votes answers also provide relevant insights) | |
| May 23, 2017 at 11:59 | history | edited | Frank Puffer | CC BY-SA 3.0 |
Tried to improve problem description
|
| May 23, 2017 at 11:54 | comment | added | Frank Puffer | @gnat: No the code is not too complicated. And it is just an example. I am generally asking if the reviewer is responsible for presenting a solution. | |
| May 23, 2017 at 11:50 | review | Close votes | |||
| S May 25, 2017 at 18:01 | |||||
| May 23, 2017 at 11:39 | answer | added | Ewan | timeline score: 2 | |
| May 23, 2017 at 11:27 | comment | added | gnat | Possible duplicate of What to do when code submitted for code review appears to be too complicated? | |
| May 23, 2017 at 11:26 | comment | added | Ewan | post the class? | |
| May 23, 2017 at 11:21 | history | asked | Frank Puffer | CC BY-SA 3.0 |