Merge then review

Comments

• GBU_28@lemm.ee ⁨1⁩ ⁨year⁩ ago

  Let the users do the testing

  source
• RenegadeTwister@lemmy.world ⁨1⁩ ⁨year⁩ ago

  [deleted]

  source

  • MashedTech@lemmy.world ⁨1⁩ ⁨year⁩ ago

    Modern agile is just waterfall with steps.

    source

    • DrMango@lemmy.world ⁨1⁩ ⁨year⁩ ago

      Wait… waterfall is waterfall with steps 🤔

      source

      • -> View More Comments
• vrkr@programming.dev ⁨1⁩ ⁨year⁩ ago

  Something like that literally happened to me yesterday. I reviewed one PR, then some Important Guy came in and said:

  • it is nice you reviewed my work, but we need to push this to production right now.
  • just fix these things, I described you how. Just copy/paste these snippets
  • these are cosmetics, I don’t care
  • "cosmetics", huh?
  • gfy and push this to production right now
  • well, ok

  Of course, lack of these “cosmetics” caused crash in production.

  source
• Blackthorn@programming.dev ⁨1⁩ ⁨year⁩ ago

  Probably unpopular opinion, but peer reviews are overrated. If coders are good AND know the project, the only thing you can do in a PR is nitpicking. They are more useful for open source collaborators because you want to double-check their code fits with the current architecture. But people here are reacting as if peer reviews could actually spot bugs that tests can’t catch. That happens rarely unless the contributor is junion/not good.

  source

  • pomodoro_longbreak@sh.itjust.works ⁨1⁩ ⁨year⁩ ago

    Peer reviews can catch bugs that tests can’t catch.

    I won’t disagree that peer reviews are overrated, but they’re a great way to train and onboard less experienced devs (who are just more fun to work with, anyway). Like I’m a platform dev, so I don’t have a “home” project - if I had to know every project before I opened a PR for it, I’d get hardly any work done. Review help other knowledge experts weigh in on my changes.

    Anyway, one case for being pro

    source
  • reverendsteveii@lemm.ee ⁨1⁩ ⁨year⁩ ago

    I operate from the presumption that code’s first job is to be as easy for a human to understand as possible. It should clearly communicate what it’s attempting to do. If your code isn’t written so that your colleagues, or you 2 years from now, can read it and understand it, it’s bad even if it’s whip tight, fits all the AC and has 100% test coverage with a perfect mutation score. That’s what I focus on when I review code: does it communicate intent semantically. Code that can be understood is code that can be reused, optimized, altered when use cases change, generalized out into even more reusable code, and provide insights that technically perfect but incomprehensible code can’t. I, like you, assume that the coder knows what they were trying to do and how to test for it, so that only gets a cursory glance to spot common errors like missed nullables, inverted conditionals and shit like that. I look at it from the perspective of “If I had to add functionality to this, could I do so easily”. Because I’m gonna one of these days.

    source
  • muddi@hexbear.net ⁨1⁩ ⁨year⁩ ago

    Nitpicking can be automated by a linter, then reviews can actually sit back and review more important things like high-level design and scalability

    as if peer reviews could actually spot bugs that tests can’t catch

    There can’t be bugs if there are no tests to catch them! Ofc you can also automate test coverage standards. But PRs are sometimes the only way to catch bugs, even and especially with senior devs in my experience bc they are lazy and will skip writing tests, or write useless or bare minimum tests just to check off code standards and merge on ahead

    source
  • the_artic_one@programming.dev ⁨1⁩ ⁨year⁩ ago

    If coders are good AND know the project

    Those are some pretty big ifs.

    source

    • Blackthorn@programming.dev ⁨1⁩ ⁨year⁩ ago

      Code review can’t fix incompence though. I lost count of how many times my boss told me “review that PR well because X is not very good”. Also my point is that they are overrated, not that they are useless.

      source
• MeanEYE@lemmy.world ⁨1⁩ ⁨year⁩ ago

  Am not sure I disagree but I don’t agree completely. It’s insane to merge things that go to production without testing. However at the same time I don’t like continuous integration one bit. Open source mantra is great in my opinion. Release early, release often. If code chews some important data, have a test version of it that needs to run some time before it gets pushed to production.

  Delaying merge of someone’s code means branches get further and further apart. At the same time code in main branch gets fixed and tested the most. I would merge often but only full features. None of the half-done stuff. Let humans test it a bit before it reaches target audience. That is usually good enough to catch most bugs. Those that do happen to leak into production are easily fixed since you have fast development cycle and deployment pipeline. And backup frequently.

  source
• Diplomjodler@feddit.de ⁨1⁩ ⁨year⁩ ago

  Test it? Meh. Just ship it.

  source

  • agilob@programming.dev ⁨1⁩ ⁨year⁩ ago

    It compiles = it goes to prod!

    source
• aaaaaaadjsf@hexbear.net ⁨1⁩ ⁨year⁩ ago

  “Xtreme programming practices”.

  Lmao what!

  source
• Locrin@lemmy.world ⁨1⁩ ⁨year⁩ ago

  Does he work at Rivian?

  source
• ikidd@lemmy.world ⁨1⁩ ⁨year⁩ ago

  The “send it” school of PRs.

  source
• SVcross@lemmy.world ⁨1⁩ ⁨year⁩ ago

  Didn’t knew KenM was on LinkedIn.

  source
• schnurrito@discuss.tchncs.de ⁨1⁩ ⁨year⁩ ago

  I mean this is basically a wiki, isn’t it.

  source

  • Shinji_Ikari@hexbear.net ⁨1⁩ ⁨year⁩ ago

    A typo in the first paragraph of the article in a wiki wont make the 5th paragraph tear down the entire wiki.

    source

    • schnurrito@discuss.tchncs.de ⁨1⁩ ⁨year⁩ ago

      you’d be surprised how complex MediaWiki syntax is nowadays, there are many ways to break things on a wiki

      source
• Devi@beehaw.org ⁨1⁩ ⁨year⁩ ago

  This explains what is going on a facebook.

  source
• Space_Racer@lemm.ee ⁨1⁩ ⁨year⁩ ago

  As a SOC auditor you programmers are going to make me scream at the exceptions you guys cause.

  source
• SonnyVabitch@lemmy.world ⁨1⁩ ⁨year⁩ ago

  Pete was Heard, but was he Listened To?

  source
• Skates@feddit.nl ⁨1⁩ ⁨year⁩ ago

  That’s nice but it goes against our quality standards and the international quality standards we are charging the client extra for adhering to, the line you’re trying to merge into is stability and needs CCB approval for the merge, and the client has specifically requested only showstopper-level bugs be addressed for stability lines. Pack your shit, you’re gonna have a wonderful time posting this crap on LinkedIn instead.

  2 days before, at OP’s former job

  source
• null@slrpnk.net ⁨1⁩ ⁨year⁩ ago

  The subtle Linux-flex in the screenshot.

  source

  • TrickDacy@lemmy.world ⁨1⁩ ⁨year⁩ ago

    Huh

    source

    • jack@monero.town ⁨1⁩ ⁨year⁩ ago

      Curse ore

      source

      • -> View More Comments
• petrescatraian@libranet.de ⁨1⁩ ⁨year⁩ ago

  @agilob code is like wine. You let it out in the cold and it gets better over time by itself.

  source
• chicken@lemmy.dbzer0.com ⁨1⁩ ⁨year⁩ ago

  I wonder how many programmers out there have intentionally set out to subtly sabotage the system. Anyone doing that, good luck to you.

  source
• halvo317@sh.itjust.works ⁨1⁩ ⁨year⁩ ago

  You guys review?

  source
• coloredgrayscale@programming.dev ⁨1⁩ ⁨year⁩ ago

  It can work if you have a test zone and only a small amount of people work on a given code base.

  Also checks to ensure the code compiles and tests pass before merging, as some quality gateway.

  source
• looseanus@lemm.ee ⁨1⁩ ⁨year⁩ ago

  deek.chat

  source