6 ms·

Sure there are a 1% of megaprojects that require additional process, but for the rest PRs are a method to control code quality socially. They introduce delays a

• olliej 5y ago
  No, every project I have worked on has review gated commits. It is a /basic/ step in ensuring that a project maintains a high quality codebase. Review does cause delays, because reviewing takes time, but we've generally found that the speed "gained" through poor change control is more than made up for through bad code. Review also shouldn't be causing ego antagonisms. You're coworkers. You have to be able to work together. That's called having a job. Every project I have worked on in a professional setting has had mandatory code review, with review acting as a gate. That's been required in order to maintain software quality despite the quality of engineers I've worked with.
  • dhzhzjsbevs 5y ago
    People seem to have forgotten what code was like back in the bad old days. Fuck that dogpile of bullshit. You've never seen a 1200 line diy function to parse XML have you?
    • Nursie 5y ago
      I apologise if I left you with that one when I left my first job in 2003! :)
      • jenscow 5y ago
        When I joined in 2006, someone had replaced it with 21 regular expressions.
        • tleilaxu 5y ago
          It's alright, when I started there I turned those 21 regular expressions into a single enormous regular expression that could only be understood at the time of writing, and never understood by mere humans again. shudders
  • jseban 5y ago
    > Review also shouldn't be causing ego antagonisms. Yeah but the tool itself is antagonistic, because it imposes a workflow of open source, a workflow that also is antagonistic with clear and absolute power. So using that tool suddenly brings that antagonism and power into a team which is supposed to be 100% collaborative, and it also only does it periodically and with random and different people in power. It's not hard to see how that becomes somewhat complicated.
    • actually_a_dog 5y ago
      Anyone who can't handle having their code reviewed before it goes into production is not welcome on my team. You can call that antagonistic or not, but I have always found that the best engineers are the ones who are able to separate criticism of their code from criticism of themselves. Get that type of people on your team, and code review is no longer antagonistic: it's just an egoless feedback process that improves the end product, which is what all members of an engineering team should be working toward.
      • jseban 5y ago
        I'm not criticising the review itself as much as rather the PR workflow and the tools of github, especially the blocking mechanism. It's my opinion that collaboration should happen much sooner rather than being pushed to the end with the review of a PR, and that you should have designated people who have the power to sign something off as production ready. If a team inside a company want to gather inspiration from the open source world, they should look at how the owner teams of open source projects work internally, not how they work together with outside contributors. And, it's a very common "solution" in tech to just simply not have any feelings, but people have feeling and that you can't turn off, and that's a very important contributor to work satisfaction.
        • actually_a_dog 5y ago
          There is nothing about github hat prevents you from working that way. I don't see what your issue is. But, I will say that I don't want people who can't separate criticism of their code from criticism of themselves on my team. You can certainly have whatever feelings you want, as long as it doesn't get in the way of producing the best possible product, all things considered.
  • berkes 5y ago
    You can have perfectly fine, high quality, peer reviewed code without PRs. Only if some regulation requires sign offs from e.g.other depts. PRs are inevitable. In all other situations they are at best an inneficient workflow and at worst a Kafkaesque circus. Peer programming, daily checkups, a rock solid CI, and, above all, trust in the professionalism of your team are some ingredients for high quality, high throughput software development. This is not an opinion. It's a scientifically proven fact. As laid out in the book Accellarate.
    • jseban 5y ago
      > Peer programming, daily checkups, a rock solid CI, and, above all, trust in the professionalism of your team are some ingredients for high quality, high throughput software development. Absolutely agree, the collaboration should happen sooner in the process, and I would add that the team should probably also have made at least a high level proposed solution together before the work even started.
    • hiptobecubic 5y ago
      Peer programming is just code review again, except now you're only allowed to write when two people are available.
      • berkes 5y ago
        Indeed. The point was not that "review" is bad, or should be avoided. But that a Pull-Request is a poor way to do that review. Reviewing is crucial, IMO.
  • chiefalchemist 5y ago
    Agreed. I'd like to add that PRs are not only a means to review code to improve that code. They're also way for new hires and jr devs to learn new things, how the hive mind thinks, etc. This is, ideally, today's PRs help improve tomorrow's code as well.
  • hedora 5y ago
    Have you ever worked on a new codebase? I've found PR's become important approximately when feature momentum starts to drop.