Questions tagged [code-reviews]
This tag is for questions about the practice of code review and code walkthroughs. For reviews of existing, working code, please see http://codereview.stackexchange.com
369 questions
19
votes
7
answers
5k
views
Debugging a performance issue, do I commit the timing code?
I am working in small company, having a lead position in a group of 5. We are developing a C++ application. One requirement is that it needs to run fast.
Today, I noticed one function, say f1(), and ...
- 301
2
votes
5
answers
751
views
Is Exception caught in the Service class a matter of preference?
In a Java EE legacy project, almost all the DAO and Service classes are written in a a way that DAO level does not catch any exception and instead the service classes catch(Exeption e) in all of their ...
- 1,935
6
votes
8
answers
912
views
What to say in a code review when I don't know how it could be improved?
I've been asked to review some code changes and when I see the code, well... I just don't like it! There must be a better way of doing this, but I can't think of one off the top of my head.
So I ask ...
- 177
3
votes
3
answers
171
views
Is Each Table Columns on Separate Pages Considered Duplication in code?
Each table is on separate pages. Is it considered duplication (written in Angular)?
The DevOps team ran SonarQube and detected the th tags as code duplication. How can I explain to them that it's not ...
- 141
-1
votes
3
answers
162
views
Finding a good development and review strategy
I will describe two development and code review processes, both describing the upsides and downsides to each version. Ideally we want to have a process that has no downsides and as much upsides as ...
- 11
0
votes
1
answer
231
views
Our php codes base has 6 different ways to do INSERT … ON DUPLICATE KEY UPDATE, how do I fix it?
Our php codes base has 6 different ways to do INSERT … ON DUPLICATE KEY UPDATE. It happened over years because the php framework is evolving and my team members come and go, although I won't say we ...
- 3,305
38
votes
13
answers
10k
views
How to give feedback on a badly reviewed PR
In my current workplace, I act as lead developer and architect in a team of software developers.
The general policy for merge/pull requests is that 1 person has to review and approve the request. This ...
- 525
0
votes
4
answers
636
views
Are there signs of bad code reviews? [closed]
I have been working almost for a year in my current place. It is a product team over some domain.
Here are some example reviews I am getting every now and then (Scala).
Removing unneeded wrapping ...
- 125
17
votes
8
answers
7k
views
How to be (more) critical during Code Reviews? [closed]
I'm a Software Engineer who sometimes need to review the code of my fellow team members. I often look at the source code and think; this looks fine. I'm having a hard time to think critical about it. ...
- 289
24
votes
7
answers
7k
views
Code review from domain non expert
My team has a mixture of specialties, there's some overlap however. When some commits are done from person A (who is expert in some domain) and person B (who is not expert in that domain) I wonder if ...
- 507
0
votes
4
answers
222
views
Should we create a new function for every multiple conditions?
I have the existing code provided below:
if (!$existing_records && !$has_required_field) {
return 'Skip Update, no records found';
} elseif (!$existing_records && $...
6
votes
7
answers
4k
views
Test A requires reviewing other files to prove it is correct. Test B doesn’t but has more than one assert
Say you want to test an update capability across a CRUD api.
Test A updates a field, queries it, and asserts that it now has the value from the update. Knowledge about the starting value (and that it ...
- 120k
3
votes
5
answers
740
views
How do I find indicators to show that code review is improving quality of a code base?
I was asked to review and handle merge request for a code base, which has been contributed by dozens of programmer with basically no regulation (or perhaps there was but nobody follows), so I set up ...
- 147
2
votes
0
answers
119
views
Should the usage of an needlessly mutable type by pointed out in code reviews, even when multi-threading is not a concern? [closed]
None of my team's programs care about multithreading, parallelisation, async, or anything else that benefit from immutability. However, immutability is clearly in fashion at the moment and I'm ...
- 1,015
-2
votes
4
answers
279
views
How do I find a good balance in the number and sizes of pull requests I raise? [closed]
Background
I've been working with a team of 6 for the last couple of months doing enterprise software development. About half the team is very new (2-6 months). We loosely follow Kanban and manage/...
- 47