• kamen@lemmy.world
      271·
      6 months ago

      The pipeline should handle formatting. No matter how you screw it up, once you commit, it gets formatted to an agreed upon standard.

      • sunbytes@lemmy.world
        2·
        6 months ago

        Some diff tools don’t handle indentation by default.

        So if you add a wrapper, it counts everything inside it as “changed”

        • kamen@lemmy.worldEnglish
          2·
          6 months ago

          Pre-commit hooks is a common approach to this, so that whatever is committed gets processed. Another possibility would be to set a bot on the repo to do automated commits after human-made ones, but that can get a little noisy.

  • jjjalljs@ttrpg.network
    49·
    6 months ago

    There was a guy I worked with that tended to want to unilaterally make sweeping changes.

    Like, “we need the user to be able to enter their location” -> “cool. Done. I also switched the dependency manager from pip to poetry”.

    Only a little bit of exaggeration

    • remotelove@lemmy.ca
      271·
      6 months ago

      Some people, like me, are not built to be developers. I can sculpt code in any language I need for whatever problem I need to solve, but maintaining code over a long period of time, with others, is not my thing.

      The drive to do additional changes is just too high and the tendency for typos or obvious logic errors is too common. (There is one little improvement. It’s right there. One line up. Just change it now while you are in there…)

      I am not stupid and regard myself as a decent engineer but my brain is just wired in a more chaotic way. For some things that is ok. For developing code on a team, not so much.

      Security is the field I am most comfortable with because it allows for creative chaos. Rule breaking is encouraged. “Scripting” is much more applicable and temporary.

      • Faresh@lemmy.mlEnglish
        8·
        6 months ago

        When using git and are working on a feature, and suddenly want to work on something else, you can use git stash so git remembers your changes and is able to restore them when you are done. There is also git add -p this allows you to stage only certain lines of a file, this allows you to keep commits to a single feature if you already did another change that you didn’t commit (this is kind of error prone, since you have to make sure that the commit includes exactly the things that you want it to include, so this solution should be avoided). But the easiest way is when you get the feeling that you have completed a certain task towards your goal and that you can move on to another task, to commit. But if you fail you can also change the history in git, so if you haven’t pushed yet, you can move the commits around or, if you really need to, edit past commits and break them into multiple.

        • howrar@lemmy.ca
          7·
          6 months ago

          Instructions unclear. Stash is 35 tall and I’m scared to look at what’s been fermenting at the bottom.

        • remotelove@lemmy.ca
          6·
          6 months ago

          I do. Also, I am old(ish) so I have had many years to come to terms with what I can do well and where I struggle.

          In this case, I didn’t want to use it as a crutch or an excuse. After reading the number of awesome replies this morning, I realized I should have probably framed my comment differently.

          People here put some real time and effort into giving some solid advice and I didn’t expect that.

          Edit: As a pure example, this is the third or fourth edit of this comment. Words are challenging, and programming is very similar in that regard.

          • UckyBon@lemmy.world
            3·
            6 months ago

            Thank you so much for your reply. Your comment was so recognizable to me, and I’m in the process of being diagnosed with ADHD.

            Edit: I want to say that I do feel sorry for asking such a personal thing about you. I’m not young either, but just now coming to terms with it and figuring out that this is the reason why everything I do goes as it does. To recognize it in the wild is absolutely weird.

            • remotelove@lemmy.ca
              2·
              6 months ago

              No worries! I am generally very open about it. (Your comment was recognizable to me, actually.) There is a very specific non-malicious bluntness that comes with the condition, actually.

              But yeah, you have been practicing dealing with it your entire life. Treatment just helps a ton.

      • Avid Amoeba@lemmy.ca
        5·
        6 months ago

        I tell my young developers - the primary goal in software engineering is maintainability. Code reuse, encapsulation, abstraction, and myriad other concepts all contribute to ease of maintaining source code over the long term. Maintainability allows for easier, predictable feature addition and removal, with fewer changes, and by different people. You’re also a different person than the one you were months or years ago when it comes to software.

        • jjjalljs@ttrpg.network
          3·
          6 months ago

          Did I already post in here about how he wrote a custom DSL instead of using the standard widely used ORM we use everywhere? Maintainability nightmare.

          He doesn’t work here anymore and now I have to either figure it out or rip it out. So far it looks like “rip it out” because it took less than an hour to swap in the orm, and now there’s just a lot less code needed.

      • nutt_goblin@lemmy.world
        2·
        6 months ago

        I’m the same way. Chasing code changes through the codebase then fighting with an edit rebase stack trying to explode them into less-interlocked changes.

        It doesn’t always work, sometimes I am just committing a giant blob of changes at work on my project I near-solo maintain 💀

      • howrar@lemmy.ca
        2·
        6 months ago

        Very relatable. Especially when it’s less effort to make the change than it is to try and ignore it. But it’s understandably harder for those who are reviewing your work.

        • remotelove@lemmy.ca
          1·
          6 months ago

          It’s even more cyclical. I usually can’t remember the reasons why I made the change to begin with.

      • jjjalljs@ttrpg.network
        31·
        6 months ago

        It could be!

        But part of working as a professional on a team is communicating and achieving consensus. Just trying to make a change like that out of the blue is poor form.

        Also consider the opportunity cost: we had planned on getting XYZ done this week, and instead he spent a few hours on this and dragged a few people into a “do we want to change to poetry right now?” conversation

    • BrianTheeBiscuiteer@lemmy.world
      1·
      6 months ago

      That wasn’t me, but that also used to be me. I learned to pick my battles, especially with complex code bases, and tried to keep scope creep in the name of improvement to like a dozen lines (provided it was fully tested).

      • jjjalljs@ttrpg.network
        2·
        6 months ago

        I think it’s definitely a thing most people grow out of when they gain experience.

        My boss told me about how when he was new he rewrote a whole chunk of the front end. His boss gave him a talking to about how you can’t just go and do that when you’re working with a team.

        At an old job I just opened a PR to apply a code formatter to an internal project I wasn’t even a routine contributor to. PR was rejected and I learned a valuable lesson about talking and getting buy-in before making sweeping changes.

  • swordsmanluke@programming.dev
    11·
    6 months ago

    Net removal of 1500 LoC…

    I’m gonna make you break this up into multiple PRs before reviewing, but honestly, if your refactoring reduced the surface area by 20% I’m a happy man.

  • Olgratin_Magmatoe@lemmy.worldEnglish
    8·
    6 months ago

    My first PR at my current job was about 130 files for the front-end, and about 70 for the backend. This hits close to home.

  • JATtho@sopuli.xyz
    81·
    6 months ago

    Please, no, I get flashbacks from my 6-month journey (still ongoing…) of the code review process I caused/did. Keeping PR scope contained and small is hard.

    From this experience, I wish GitLab had a “Draft of Draft” to tell the reviewer what the quality of the pushed code is at: “NAK”, “It maybe compiles”, “The logic is broken” and “Missing 50% of the code”, “This should be split into N PRs”. This would allow openly co-develop, discuss, and steer the design, before moving to nitpicking on the naming, formatting, and/or documentation details of the code, which is likely to drastically change. Drafts do work for this, but the discussions can get uncomfortably long and convolute the actual finishing of the review process.

    Once both reviewer(s) and the author agree on the code design, the “DraftDraft” could be collapsed into a link in an normal Draft to be mocked next. The scope of such draft would be limited by the earlier “DraftDraft”.

  • tatterdemalion@programming.dev
    82·
    6 months ago

    This does seem like a potential issue if the PR is itself implementing more than one vertical slice of a feature. Then it could have been smaller and there might be wasted effort.

    If the patches are small and well-organized then this isn’t necessarily a bad thing. It will take more than one day to review it, but it clearly took much more time to write it.

    • Throwaway@lemm.ee
      4·
      6 months ago

      True, but at the same time its very possible to go too small. A bunch of one line code reviews can really slow progress easily.

      • tatterdemalion@programming.dev
        2·
        6 months ago

        Right but it’s pretty rare that a tiny PR actually accomplishes a valuable user story.

        So my point is just that lines of code is mostly irrelevant as long as it’s organized well and does no more than necessary to accomplish the agreed upon goal.

  • phoneymouse@lemmy.world
    4·
    6 months ago

    Love it when my coworkers reformat the code style, making it nigh impossible to understand what they actually changed, while greatly inflating their “contribution.”

    It also blows away the git blame, making it hard to know who actually changed that one critical line of business logic 3 years ago that you need to understand before trying to fix some obscure bug.

    I have one coworker who does this constantly and if you just looked at git blame, you’d think he wrote the entire code base himself.

    • ursakhiin@beehaw.org
      4·
      6 months ago

      Human made changes is likely not what caused this image to occur.

      111 files with that kind of change count is most likely a dependency update. But could also be that somebody screwed up a merge step somewhere.

      • ulterno@lemmy.kde.socialEnglish
        1·
        6 months ago

        Or maybe their IDE had a different auto indent config and they saved it all, then committed it all without checking the diff or the status.

          • ulterno@lemmy.kde.socialEnglish
            0·
            6 months ago

            I do like the idea of mandating git clang-format as the Kate project has.
            That way the other devs don’t need to change their own IDE settings to comply.

      • shiftymccool@programming.dev
        1·
        6 months ago

        The only way I see that is a dependency update is if you’re versioning your node_modules or <insert-folder-here> which is generally a no-no

        • ursakhiin@beehaw.org
          1·
          6 months ago

          Many organizations vendor packages in the repo for a number of different reasons and languages. Not just for node.