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

Filter by
Sorted by
Tagged with
-1
votes
2answers
64 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 ...
-2
votes
1answer
191 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 ...
6
votes
2answers
416 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 ...
3
votes
3answers
408 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 ...
6
votes
4answers
700 views

Code reviewing nitpicks for readability [actually for DRY]

I was doing a code review today and noticed that the code would work but could have easily been made more readable. The example in particular was roughly as follows: def group(x): GROUP_ALPHA = (...
11
votes
4answers
625 views

How to review sql changes more effectively? [duplicate]

From my experience, sql code changes almost always tend to be NOT incremental: someone creates a new stored procedure, or modifies an entire embedded sql query for optimization purposes, or creates a ...
5
votes
2answers
419 views

How To Get A Junior Developer To Stop Bikeshedding [closed]

I'm having an issue with a junior developer bikeshedding PRs. They are the primary approver for one repository. Things that should be up for an hour or minutes on PR spend days sometimes with back-and-...
1
vote
1answer
271 views

Request changes to a commit message during code review on GitHub

I am trying to teach good engineering practices to my team, and in particular, walking them through the basics of PRs and code reviews on GitHub. One of my team members has sent me a request to review ...
-2
votes
1answer
242 views

What is the meaning of 'code vetting' in Software Engineering? [closed]

I saw the term code vetting for the first time in a gerrit review page. What does it mean? The line I came across: "No XYZ check was done. However, if this project is intended to be shipped in ...
-1
votes
1answer
84 views

Should one create code review for an incomplete unfinished change? [duplicate]

Had this happened: A developer make a new feature, but not working, since last year, total code change is 100+files and 10k lines. Until recently, with help of another developer, get it to work. ...
2
votes
0answers
180 views

Gerrit: Working around the “single-change:single-commit” principle. Need Feedback

I have been using Gerrit in my work environment and I find it really amazing and powerful because of its elegant use of git's branching features to create virtual branches for code reviews (the "refs/...
0
votes
3answers
365 views

Code Design: Repeatedly doing the same Boolean check with Stream Writer

I have several areas in a program where I am doing the following check on the same two booleans, but each spot has different text being written to a file via Stream Writer based on the value of the ...
1
vote
1answer
181 views

How do I do code review for an unfamiliar code base? [duplicate]

I work with a small team of software engineers. Each one of us handle different projects without much interaction. We all report to the same supervisor who more or less understands the requirement ...
-1
votes
3answers
248 views

Code design: introduce new method vs add optional parameter to existing method

In JavaScript (ES6), I can have optional function parameter with default value. So to add a new behavior to the existing code base, I could either introduce new method(s) or extend the existing method ...
6
votes
5answers
641 views

Assigning people to code review

My team has a problem with making code review. Everyone is constantly busy with their tasks and only few of developers do review on a daily basis. As every merge request needs 2 approvals, some of ...
0
votes
1answer
38 views

Removing unused directives from a file which is not related to the pull request [duplicate]

I'm actually working on the web application which was made using technologies such as asp.net core, c#, angular, ef core. For the version control and CD we use Azure DevOps. As a team we try to follow ...
2
votes
2answers
281 views

How to distribute code reviews workload in a small team?

I have a small team where I'm largely the only person who reviews what goes into production and makes sure that the code introduced doesn't contradict business logic, no bugs etc.. I know a lot of ...
2
votes
1answer
153 views

PHP - Dynamically apply parsing rules to text, and save to dynamic fields

OK, so I have been at this problem for the last few days, and I must admit - I am stuck. I am trying to make a web application, where users can upload documents or send inbound e-mails. Each ...
1
vote
1answer
226 views

PHP - Designing a rule based parsing engine

Dynamically apply methods/"rules" to documents I hope someone here can help me/guide me in the right direction. I am currently creating a web application, where users can import a text file, and then ...
2
votes
1answer
217 views

How to set up code reviewing in a naive team? [closed]

I am working somewhere where programming is an important part of the job, but where code review is something nobody ever heard of. Being kind of enthusiastic about programming, I've seen a lot of ...
65
votes
12answers
10k views

Having a flag to indicate if we should throw errors

I recently started working at a place with some much older developers (around 50+ years old). They have worked on critical applications dealing with aviation where the system could not go down. As a ...
1
vote
0answers
68 views

Perspective based reading - Artifact created

From what I have learned, in Perspective based reading, the document/code/etc., is inspected from different perspectives. We consider roles like software designer, tester or user. Each role follows ...
0
votes
1answer
111 views

Chain function calls which return error codes or 0 on success in C [closed]

I used to call functions which returns int error code or 0 on success like this: int tmp = function_a() ?: function_b() ?: function_c(); if (tmp) handle_error(); Now I'm ...
2
votes
4answers
385 views

Putting extra code review burden on team because of refactoring

I am reading Refactoring by Martin Fowler, and working in the patterns suggested into my coding. This has caused me to put up two code reviews for my service which are significantly lengthy, which are ...
-1
votes
1answer
26 views

How to use pull requests for code reviews with a shared remote repository?

I'm looking to improve the processes in our team. We currently are pretty informal: "I've finished feature X. Can you review my code?". This is not scaling up well now that we're busier - it's hard ...
1
vote
2answers
392 views

Peer review of unit test

What is a good strategy for Q/A Engineers peer reviewing Unit tests as part of the software development life cycle? I think that there are, indeed, things that I should check regarding the unit test ...
12
votes
4answers
1k views

How to make better code reviews when pull requests are large?

Disclaimer: There are some similar questions, but I didn't find any which touch specifically the problems you face while reviewing a large pull request. Problem I feel my code reviews could be done ...
3
votes
3answers
1k views

Are there any drawbacks to using a nested class instead of declaring a new one?

I'm doing code review on a change my co-worker made to our Java application, and I've found something I'm not very familiar with - a nested class. From reviewing the code, it seems like the nested ...
0
votes
1answer
124 views

How to handle complete rewrite of a file in regards to code review

My situation is this: I am contributing to a Symfony/PHP project. I had to completely reskin one page in the application. The controller logic is almost totally unchanged. I only had to change the ...
4
votes
3answers
336 views

Making pull requests easier to review while practicing XP and refactoring

I practice TDD and refactor aggressively. Recently some colleagues have complained that the resulting pull requests are difficult to review, or would require a lot of regression testing. When I ...
53
votes
11answers
11k views

Agile Practices: Code Review - Fail the review or raise an issue?

At the end of a 2 week sprint and a task has a code review, in the review we discover a function that works, is readable, but it's quite long and has a few code smells. Easy refactor job. Otherwise ...
2
votes
1answer
168 views

How to review hundreds of commits and formalize merge request(s)

At my work every line of code must be reviewed and this review is formalized in a merge request. Now we are a facing a situation were one developer developed a prototype for a new product which is ...
14
votes
6answers
2k views

Clean Code - Should I change the literal 1 to a constant?

To avoid magic numbers, we often hear that we should give a literal a meaningful name. Such as: //THIS CODE COMES FROM THE CLEAN CODE BOOK for (int j = 0; j < 34; j++) { s += (t[j] * 4) / 5; } ...
3
votes
6answers
3k views

How to avoid code duplication in if else condition?

How can I avoid duplicate use of doSomethingCommon() in the following block of code? doSomething(); if (conditionA) { doSomethingSpecificToConditionA(); doSomethingCommon(); } else if (...
13
votes
5answers
1k views

Getting ALL developers do code reviews

I'm a software developer in a 7-8 developers team. We've been doing code reviews for quite some time now and code quality has improved over time. However I recently noticed that some developers are ...
3
votes
4answers
933 views

Is “Let more people able to review” a valid reason to “write c++ as if Java” at some degree?

According to Should we avoid language features that C++ has but Java doesn't?, I know it is horrible to write C++ as if Java, mostly because it drops the beneficial features of C++ languages. But ...
-1
votes
1answer
164 views

How to Gain full Confidence in Pull Request builds

Having confidence means Pull Request builds (in Jenkins) will finally allow you to merge the code after a Pull request builds successfully and is reviewed. You're 100% confident that it can go into ...
0
votes
4answers
429 views

Magic __init__ and regular init methods in a python class

Is naming a regular method init only confusing or is it an error? class A: def __init__(self, x): self.x = x def init(self, y): self.y = y**2 def reset(self): ...
0
votes
1answer
3k views

Easily exclude commits on a merge based on some condition

My eyes are destroyed. I am using a screen reader. We are using git as our version control system on our project. We host our own server and use ssh to access it. We put each module under separate ...
2
votes
4answers
339 views

Why chain assignments to clear variables?

Why do some developers in C for example chain together assignments of variables like this: i = j = k = 0; or sometimes formatted like i = j = k = 0; Is there some underlying complier optimization ...
21
votes
6answers
7k views

How do I document my code for minimum time review? [closed]

I want to document my code such that there is minimum need for reading and browsing the code again months later. I know that there are different types of documentation (in source code and outside, ...
0
votes
2answers
4k views

changing the value of String object in java

I have to set the value of a string object "result" depending on the results of different methods and different if/else conditions. In the end, there would be one (last value set) in the string that i ...
11
votes
4answers
1k views

How do I wrap a service so it is simpler

We have a dependency to a third-party service which exposes a gigantic interface of which we only need like 3 methods. Additionally, the interface changes frequently... I've decided to wrap the ...
10
votes
3answers
2k views

How to give feedback after code review process

I am currently reviewing some of the code of junior developers who just joined my team, I am wondering how should I deliver the output of this review: Should I fix the code myself? Should I give them ...
4
votes
1answer
189 views

What's a reasonable in-house code audit policy?

In a company of about ~100 developers, and ~2 pen-testers, a product is being developed. There's a good amount of diverse code coming in via github PR requests every day. The goal is to review as ...
3
votes
3answers
326 views

Can you consider a bug fixed when the underlying cause was not fixed?

Let's say there is an extensible system in place, which uses an algorithm that requires that the elements it operates on are unique. The signature to call this algorithm looks like this: function ...
25
votes
4answers
2k views

How to handle a TODO in a pull request?

When I review the changes in a pull request, I sometimes stumble upon a comment with a "TODO" note which may be there for different reasons, in our case mostly because of: the solution used to solve ...
3
votes
3answers
1k views

two level code review using git pull request in BitBucket

The following procedure: We have Development branch and Developer create feature branch from Development Work and commit in this branch Rebase from Development Create pull request Code review Merge ...
5
votes
5answers
549 views

Check if variable is not null before setting to null

One of our team likes to write code like this (C#): if (_someVar != null) { _someVar = null; } Rather than simply: _someVar = null; I've never used the former as I always assumed it was ...
0
votes
3answers
260 views

What's an effective strategy to mark code as reviewed?

I am reviewing a big C# code base and thought on how to mark it as reviewed when it is, in order to not review it twice. Currently I have a ReviewedAttribute which I use to decorate members that have ...

1
2 3 4 5
7