• Solemarc@lemmy.world
    link
    fedilink
    arrow-up
    93
    ·
    1 day ago

    I struggle to review a 1k line change. When people give me such big changes I normally don’t believe they’ve reviewed them either.

    • Jesus_666@lemmy.world
      link
      fedilink
      arrow-up
      16
      ·
      22 hours ago

      Try working on a codebase that’s all event-driven hexagonal CQRS with hand-crafted SQL for persistence. Add additional buzzwordy methodologies to taste.

      Adding a single property to your product means you now have to update an aggregate class, several DTOs, and several event classes and handlers before you can even think about touching the UI.

      And that’s in your main solution. There’s also at least one facade service you’ll need to make compatible and you also need to update the event simulator used for testing. The latter night involve having to touch every single line in a 2000 lines long SQL script.

      Having to go though three separate 600-2000 LOC PRs for one PBI isn’t that exotic.

    • Saganaki@lemmy.zip
      link
      fedilink
      arrow-up
      7
      arrow-down
      1
      ·
      23 hours ago

      Occasionally I do that…but only because 500 of those lines are my comments explaining everything.

        • Saganaki@lemmy.zip
          link
          fedilink
          arrow-up
          3
          ·
          10 hours ago

          I’m aware. Not always feasible. For example, had to add a custom video capture solution that captures the last 30 seconds of a process for crash handling purposes.

          You most definitely need to do add that much comments explaining the mp4 box format along with the box “hierarchy” of what is being written. Add to that MFT (h264 encode) code…

          Basically, if anything, the comments are for me for when I look back at that code.

          • MonkderVierte@lemmy.zip
            link
            fedilink
            arrow-up
            1
            ·
            9 hours ago

            had to add a custom video capture solution that captures the last 30 seconds of a process for crash handling purposes.

            Commanded from above? This smells like a noob idea.

              • Jaycifer@piefed.social
                link
                fedilink
                English
                arrow-up
                1
                ·
                8 hours ago

                This is my attempt to translate: You had to add a feature like that ? Must have been an order from your boss. That seems like a feature someone rather inexperienced and unknowledgeable would request.

                • Saganaki@lemmy.zip
                  link
                  fedilink
                  arrow-up
                  4
                  ·
                  8 hours ago
                  1. Legal has issues with using existing libraries (including MIT). Definitely idiotic, but I can’t control that.
                  2. Idea was mine.
                  3. Subprocess that captures parent process active video with a rolling buffer for crash handling purposes is absolutely necessary when trying to reproduce issues in development (Gamedev editor). If you think associating the last 15s prior to a crash with a mindump isn’t helpful for debugging, I don’t know what to tell you.