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
163
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
1
vote
5
answers
620
views
When code review becomes a formality what procedure can I take to amend it?
I am dev manager, managing 8 developers. I am facing a problem that the code review becomes more and more a formality even though we all know the code review is important. We do have code review ...
- 3,305
6
votes
5
answers
2k
views
Code review turned into rewrite, should I push my changes?
I was reviewing a pull request of a programmer that works under me, and found that in testing fixes to their code, I ended up completely rewriting it.
Should I push my changes and then explain what I ...
- 500
0
votes
1
answer
123
views
Aiding code review with an additional pull request
I am curious how others tackle the issue of needing small, sometimes trivial changes in a pull-request. The problem crops up a lot where I work. A developer completes a task and makes a pull-request, ...
- 117
4
votes
3
answers
1k
views
Should PR reviews check for code correctness?
When doing PR reviews, should the reviewer be checking that the code is correct (e.g. the logic is correct, etc.) or should the only focus be higher level concepts (e.g. architectural/functional/etc.)?...
- 1,253
1
vote
2
answers
2k
views
Workflow for splitting large pull request into multiple reviews
Nobody wants to review a large PR from one go. The issue is to split a (relatively large) feature into multiple smaller PRs. The rules of the game are
This is a single feature, which cannot be ...
- 2,631
0
votes
6
answers
304
views
Is it necessary to understand the requirements of a change in order to perform an effective code review?
Can an effective code review be performed without first understanding the intent behind the code?
- 168
1
vote
3
answers
3k
views
How to manage pull request review and approvals
In our team we have a recurrent problem that we are not able to solve. As long as the team grows, we tend to accumulated PRs at the end of the sprint / release and these become a bottleneck to advance ...
- 151
0
votes
2
answers
385
views
How to review a PR where I think the complexity is gratuitous?
I am in this situation where I have to review pull requests (PRs) from one of my colleagues, who has a very broad knowledge and years of experience, she is well respected and we all know that the ...
- 243
3
votes
4
answers
2k
views
If PRs are not being approved, when do you stop branching off of master?
I prefer to create linear git history, which can be fast-forward merged. This was a technical requirement at my previous job, which used BitBucket and required all PRs to be fast-forward-able. We had ...
- 363
2
votes
2
answers
1k
views
How to handle code reviews on a collaborative feature branch
We are using GitHub as our source code repository along with Visual Studio. We are going to start to use the GitFlow branching model, so a new feature request will be branched off and worked on in ...
- 131
1
vote
3
answers
355
views
How can we reduce the PR load on our team?
We are using microservices(read a lot of GIT repositories) and our department has 90+ java developers. All of them write code which mean there are a lot of PR`s every day. We are using BitBucket and ...
- 119
2
votes
1
answer
461
views
How to evaluate the impact of change for a single line of code or a variable?
During unit testing it is possible to estimate the code coverage to see which share of the code base is covered by the tests.
For one part (the simple calculatable one) of a risk estimation we need to ...
- 129
3
votes
4
answers
387
views
how to perform code review among different types of developer?
My develper team includes Android, iOS, PHP, Web Front, .Net developers, but every type of developer only 1 person, and they don't know other's programming languages very well.
We have setup Jenkins ...
- 149
0
votes
1
answer
165
views
Understand implementation of exponential moving average (in case of unix load average)
The UNIX load average gives 3 numbers over 1/5/15 minute time intervals. It's supposed to be an indicator of how busy a UNIX machine is. The global load average is an exponentially decaying average of ...
- 337
8
votes
3
answers
794
views
How to get code review on hobby project?
In my spare time, I like to develop various small libraries and CLI utilities that solve a real problem I have, but are also likely useful to other people with the same use case. When I say small, I ...
- 107
1
vote
3
answers
960
views
A lot of modifications in pull requests
I would like to know how manage a type of pull requests. One of my reviewers ask me for a lot and a lot of changes (3 or 4 days doing changes), all related with possible improvements in the code that ...
- 145
-1
votes
1
answer
108
views
Design suggestions for my simple data-analysis program
I need to create a program with the purpose of cross-referencing personal info from a spreadsheet(s), to check for conflicts of interest between clients of 3 different law firms. All of this client ...
- 99
39
votes
17
answers
10k
views
How can I defend reducing the strength of code reviews?
I have started in a new team. I have 20 years experience as a developer, and I have been in the role of a team lead in several projects.
Normally I am very much pro code reviews, but I ended up in a ...
- 540
33
votes
3
answers
10k
views
What is the best way to code review a work-in-progress?
I am working on a feature with a system that I am unfamiliar with. The feature is not ready, but I want to show the code to my team (who is familiar with the system) so they can give me early ...
- 1,563
1
vote
1
answer
466
views
Identifying Risks/Gotchas When Using Static
I am intending to come from the perspective of development choices, code reviews, and general testing against defined environments (Development, Test, Production, etc.). I will be using C# as the ...
- 414
9
votes
2
answers
644
views
Where is the value of online diff code review?
I recently joined a team who are using github Pull Request feature as the single code review tool.
In previous teams, I was used to reviews done on a fully checked-out code base, done using IDE tools, ...
- 1,040
5
votes
8
answers
1k
views
How to scale code reviews
My boss say we should find a way to scale code reviews at our company. As it is right now, we have about 16 software developers spread across 4 different teams/squads, but soon the company will close ...
- 175
-4
votes
1
answer
301
views
Python: Help with replacing list comprehension with generators [closed]
I have written the following code which takes a coord_list of points in a 2D coordinate system, a center and a radius and returns a list of all points from the list having distance at most radius from ...
- 181
2
votes
1
answer
1k
views
How do pull requests fit into a development process?
In my software engineering career I have never worked with pull requests. Probably because I only ever worked in relatively small teams (5-16 people) and only on projects that were structured well ...
- 368
0
votes
1
answer
396
views
Code review patterns for multiple teams working on a single product
We're scaling out development on a single product from a single team to multiple teams. What are the patterns to ensure coding style, patterns and technology is used consistently?
Is there one person ...
- 617
69
votes
8
answers
17k
views
In code review, should I ask to do a refactor outside of the scope in a pull request?
I have been studying the best practices for a code review, and I was wondering what to do in the following scenario:
During a code review, I see potential improvements, but decide that they are ...
- 817
4
votes
1
answer
945
views
Files moved and modified in one pull request
Sometimes I get to review a bunch of files that have been moved and modified. In GitHub I cannot find a way to see what has been modified. Everything looks like a new file. It makes review harder ...
- 650
1
vote
1
answer
468
views
Cyclomatic complexity vs repeated code
we've a python function that can be achieved in 2 ways
1st method
def complexity_1(x, y):
if 2 == x and 3 == y:
a=3
b=4
c = a + b
elif 2 == x and not 3 == y:
a =...
-3
votes
3
answers
239
views
Inspection code reviews, is it outdated today?
I'm a big proponent of peer reviews on every change, like with pull requests etc, however I'm questioning if this type of review alone is enough.
For one, I have trouble with reviewers 'missing the ...
- 15
28
votes
13
answers
10k
views
Ways to explain code when told it doesn't make sense
As a programmer I have found my code frequently elicits the reaction "I don't understand". Whenever I get this response I try my best to explain my code patiently, and not make anyone feel ...
- 500
-1
votes
1
answer
171
views
What is the most effective way to explain code in a code review using a pull request?
I work for a small company with only three programmers (including myself).
Our workflow is:
We write the code;
We create a pull request on Github;
We ask for code review;
We merge the pull request.
...
- 37
-1
votes
2
answers
129
views
Code Review for Automated Unit Tests [duplicate]
Writing automated unit tests is followed as a part of our development process.
We also do have an established code review process for the development code that is written.
Should the test code be ...
- 107
-2
votes
1
answer
219
views
How to react to a code pair reviewer pointing out way too many small details [closed]
I recently came to a situation where I could not really figure out how to react. During a project, in a paired code review, the software architecte began pointing out even the smallest detail of my ...
- 113
6
votes
2
answers
1k
views
How to avoid code review bottlenecks when some of the team pair program
There is a school of thought that describes code reviews as the number one priority activity for all developers in a sprint since they are either:
a) A source of unrealised value
b) A source of bugs
...
- 9,843
3
votes
3
answers
708
views
Isn't keeping the master branch intact essential for collaborating?
I've read a lot of useful articles about collaboration with git. Many do's and don'ts, etc. For example, Top ten pull request review mistakes and a nice article about git workflow The Perfect Code ...
- 151