My biggest issue would be from a security perspective. The background checks and everything are nice, but there are some systems I would just never let this service touch.
Overall, this is interesting! Wish the team behind it the best of luck.
191–200 of 238 posts
My biggest issue would be from a security perspective. The background checks and everything are nice, but there are some systems I would just never let this service touch.
Overall, this is interesting! Wish the team behind it the best of luck.
This 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.
Earlier quoted context omitted.
I'm talking about code where thee context is donain knowdledge and architecture. Reviewing things like style, performance, security, framework best practices etc is pretty easy work and rarely the bottleneck in a team in my experience. Basically: any kind of review where you could comment on a single file only, is easy. The important and difficult part of review is "Is this the right thing to do at all? Is it impleme…
I'll argue that, in an ideal world, most of the questions you're asking here should be addressed before anyone writes code. A basic spec, with "here's the idea", "here's how I plan to prove the concept viable", and a rough plan of "here are the software components I'll use" doesn't take long to put together, relative to actually coding the thing up. Having that document and getting it reviewed should answer a lot of…
I agree completely. But code review to me is the chance to pick up on those situations where the situation wasn’t ideal. And even if this is just one time of 100, that review was still more important than the remaining 100 “normal” reviews with more mundane feedback.
Earlier quoted context omitted.
Here's a question: do you get pushback about "best practices"? (Sometimes really just "standard practices".) If so, which ones in particular? And do you actually continue trying to explain why it's a good idea, or just throw up your hands with a feeling of "I gave my advice, they can ignore it at their peril". Having actually convinced people of the value of certain practices (like favoring fast focused unit tests ov…
I've gotten pushback on best practises, especially from more junior developers or developers on a tight deadline. I do C# and one common, yet trivial example is poor naming conventions. As a reviewer we have certain tools we can use to encourage change: * We can make comments low priority, so the advice is there, but it's skippable. * We can make summary comments, that are opinionated but don't expect any immediate o…
I've sometimes had trouble convincing some older programmers about using new (or even not so new but slightly 'advanced') language features like Java lambdas or Optional or generic types (juniors/interns were often aware of and happy to use the new stuff already), to the point that for specific individuals I'd just give up and focus my review efforts on other aspects. However if I ended up touching that code myself later on, or someone else did whom I was reviewing, I'd use those newer features/encourage the other person to use them if they weren't already, and if the original person ever came back to it they'd face an argument of local consistency against trying to change it back to using the old ways. It seems like the approach of longer-term encouraging and supporting a pocket of engineers pushing for better practices would actually work with this service given the ability to make notes for future reviewers. Another flaw in Code Collab was its atrocious search which made it hard to find prior reviews about a set of files.
I'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…
That was my initial reaction too, but then I thought "using such a service might encourage the code-based to be external-reviewer friendly?". That is, instead of all the shared history/context that internal reviewers have, make all that as explicit as possible, ideally in the code, or at least in good code comments. Would also go a long way to avoid the "bus factor" (or more commmon: the "sudden-quit factor").
Earlier quoted context omitted.
Disclaimer, I'm the CTO @ PullRequest. One thing to note about our service is that we are not trying to replace your code review process if it is already working well and we strongly agree that knowledge transfer is a very important part of code review. ( We actually have code review metrics as well that help encourage and reward your internal code review process. ) However what we do believe and see on a daily basis…
> Our reviewers are all highly qualified, many are maintainers of popular open source projects or work at top tech companies. Isn’t that potentially a huge problem? What if your reviewers work for my company’s competitors? I don’t want them seeing our code base. Do you have any methods to ensure that doesn’t happen?
This 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.
Earlier quoted context omitted.
>> it breaks Agile It must be "nice" working with an enterprise architect who views agile as a strict, narrowly defined thing.
He's really sharp and has keen insight into how enterprise business works. His advice saves us a lot of time and money.
Earlier quoted context omitted.
He's really sharp and has keen insight into how enterprise business works. His advice saves us a lot of time and money.
But if he wants you to open tech debt tickets, rather than spotting/fixing security issues immediately, this will not save the company time or money.