I hope it can review popular code on github.com automatically.
Code Review as a Service
81–90 of 238 posts
Re: Code Review as a Service
#82I have performed reviews on pullrequest.com, and lack of context tends not to be an issue. Code review is an interactive process, where questions can be posed to the PR authors, and PR reviewers have access to search the codebase. Customer success at pullrequest.com also provide context for the repo and organization working practices, attached at the top of PR descriptions, to help reviewers with context and to know what kinds of feedback are useful. Sast and Dast are still not sophisticated enough or widely used enough to be good enough. In-house senior and principal engineers often do not have the time to be on top of every repo and every PR in their organization, and need to outsource an extra pair of eyes. It is highly common that I am reviewing codebases that do not have any Sast or Dast, or have workarounds to silence the tools, resulting in bugs and security issues slipping through. It is very rare that I am able to approve PRs without comments, even after an internal review has taken place!
Re: Code Review as a Service
#83Except I don't believe in code reviews. Over the years I have experimented with pair programming and I think it is way better solution.
The one catch with pair programming is that some people just prefer to work alone and it is really draining to be talking constantly for couple of hours. I have modified the system to do on/off pair programming (ie. split up a little bit of work, rejoin later in the day, share what we have done, and work a little bit together).
The basic, unsolvable issue with code reviews is that the review is done after the code has already been written. As you know, the cost of fixing a problem is larger the later in the process you catch it. Pair programming aims to accomplish the correction while the code is being written.
Another huge problem is that, because the reviewer is not taking part in the development, he/she does not have the same level of understanding of what was supposed to be done.
Also, code reviewers are typically disincentivized from doing review well:
* They have other tasks to accomplish, review takes their time away from those tasks but the deadlines are not pushed automatically,
* The review tends to land at a random point in time disrupting their flow -- they have something else in mind already and they want to switch to their work as quickly as possible -- meaning they will not want to get into great detail with understanding the problem.
This causes reviews to usually be very shallow and focused on trivia. Usually, I see reviewers read the code file by file line by line, hundred times faster than it was written. It is absolutely impossible to verify a large change like that and this guarantees that they will not actually verify it thoroughly.
Yeah, you may find superficial flaws, but that's about it.
Other problems:
* The developer feels resentment because he/she thought it was all done.
* A lot of effort was spent on a wrong solution which is lost productivity.
* From project management PoV it is a problem because we can't tell how much time/effort it will take until last second.
* Reviewers feel pressure to find something so they will just report trivia if they can't find real issues.
* Reviewers tend to not want to report huge issues that would require complete rewrite because they are developers themselves and wouldn't want the same happen to them, and also because they are typically members of the same team.
* and so on.
Re: Code Review as a Service
#84And I’m guessing their reviewers work for free, like on Code Review Stack Exchange? Or are these people working for relative poverty wages overseas?
> And I’m guessing their reviewers work for free, like on Code Review Stack Exchange? No need to guess, it takes 20 seconds to check: https://app.pullrequest.com/signups/reviewer
Reviewers are well-compensated, meaning pullrequest.com can attract very high quality reviewers.
Re: Code Review as a Service
#85I'm not sure this is a good idea for enterprise startups building closed-sourced software. As a software architect, some things you just don't delegate. Code Reiews are some of the most important things I do. But (and please don't downvote me for this, I know there's an instant reaction to downvote anything blockchain related) for DAO's this would be amazing. A DAO (Decentralized Autonomous Organization) might be run…
I didn't downvote you, but I'd love to hear some situations in which this hypothetical DAO would be comfortable running code that it (where "it" means "an engineer who has a stake in the DAO and so is invested in its success") had not reviewed? How would it trust the results of this review? I'm trying to remain open-minded and curious about blockchain-related use-cases.
Re: Code Review as a Service
#86I'm all for this if the person reviewing my code will know the context, history and all the details and conversations we had as a team. But in order for that to work, I'd probably be taking most of this reviewer's time. And obviously in order for them to get up to speed with our practices, conventions, architecture, code style and whatnot, they'd probably need to start by doing a whole lot of development on our proje…
My company has used them before. I thought the same before using them. However I was surprised the level of talent they have. For a super senior dev the context that you need for most projects isn’t as much as you think. It was a net positive for us. Sped up our developer process given its so hard to hire senior devs right now. They also have domain experts. So say you are using some tech your team isn’t as familiar…
I'm one of those reviewers, and I agree with you about the talent. Not speaking about myself, but the people I've gotten to know and also the ones I have referred.
Each PR gets two reviewers from pullrequest.com, and we get to see each others' comments. One will catch stuff the other misses, and we usually support each other. It's most fascinating when we disagree on something, which so far has always led to a high-quality discussion between the engineers and the reviewers.
I've been working with them for most of 2021, and I can honestly say I'm impressed with the review comments I have seen. It's been nothing but respectful and professional. As a plus, it's made me a better code reviewer at my day job.
Re: Code Review as a Service
#87Earlier quoted context omitted.
But almost any good SA can catch all of those issues, a person doesn't need to look through the code for that.
This is very close to saying Halting problem is not, in fact, a problem: https://brilliant.org/wiki/halting-problem/
A human being doesn't solve the halting problem either.
Re: Code Review as a Service
#88Earlier quoted context omitted.
I mentioned this idea to a very knowledgeable enterprise architect. I was mentioning how much I'd enjoyed being able to use the code review process in this way. His reply: As good of an idea that is, it breaks Agile. So long as the code meets the biz-provided spec, it must be accepted, and if there are other concerns with the code, to make a tech debt ticket to address it at a later date. This, of course, horrified m…
His logic would apply to tests of all sorts. As long as the dev says the feature is done, that’s the end. And he’s wrong. As teams (or companies, or whatever unit) we get to decide our definition of “Done”. I suggest that definition include appropriate testing. And appropriate code review, which serves two purposes… catching defects and knowledge sharing.
In his world, testing the software belongs to a different team than the implementation. Differently managed, differently staffed. Not with engineers, but with testers. It's difficult to say the least to move from testing into engineering
And you can't just redefine Agile like that. It's the businesses' money, culture, and productive means. Not yours. If they want teetering software stacks with no thought given to maintainability, managed by non-tech-savvy staff, then that's what their money will buy.
We enjoy an environment catering to our needs because our orgs can afford to throw a lot more resources at better managers and better talent. As a result individual contributors can contribute not just tickets, but also to help improve the way we work. Not possible in heavily top-down org structures.
Re: Code Review as a Service
#89This entirely misses the point of code review. Say it with me: Code review is a knowledge transfer exercise. Finding bugs, security vulnerabilities, and keeping the code maintainable are merely side effects that we appreciate along the way. The primary purpose of code review is increasing the bus factor of the given piece of code and facilitating organic knowledge transfer. That's it.
In most of the projects I have reviewed for pullrequest.com, the engineering team is also doing its own reviews. We are "another set of eyes" as it were. Many of our larger reviews can end up becoming lengthy conversations between us and the team. There is definitely a lot of knowledge transfer going on. What's been truly rewarding is when someone on the team comes back to us and asks for advice on the best way to solve a particular problem.
Re: Code Review as a Service
#90I'm all for this if the person reviewing my code will know the context, history and all the details and conversations we had as a team. But in order for that to work, I'd probably be taking most of this reviewer's time. And obviously in order for them to get up to speed with our practices, conventions, architecture, code style and whatnot, they'd probably need to start by doing a whole lot of development on our proje…
Code should be written to be understood without that context. If comments and documentation aren’t enough context for the code to be understood, it probably isn’t written very well.
It is very bold statement.
Code models some real-world entities (domain). Code (completely with comments and documentation) cannot and should not document fully domain. It is context, which is needed to understand code.
Yes, simple CRUD application can have all context encapsulated, but what's about some code which models, say, some aspect of chemistry? Should this code have enough context which includes several post-grad university courses?
Or «simpler» example from my current $Job: we have a lot of code to build some models of derivative stock exchange instruments (options, futures, etc). Enough context for this code is, like, full shelf of 1000 page books. Good luck to review this code for everything but off-by-one errors if you don't work in this area for 5+ years.