81–90 of 184 posts
> For a large change, I don’t want to do a “diff review”, I want to do a proper code review of a codebase at a particular instant in time, paying specific attention to the recently changed areas, obviously every team and ticket is different, but IMO unless the person doing the review is some sort of principal engineer mostly responsible for the code at large, this does not align with I would personally consider a cod…
It seems to me it misses the mark a little -- the text is so verbose, it's easier to read the code itself.
A third (fourth?) option worth mentioning here is difftastic[0], which uses "structural" diffing (as opposed to line diffing) for more granular diff highlighting. [0] https://github.com/Wilfred/difftastic
> For a large change, I don’t want to do a “diff review”, I want to do a proper code review of a codebase at a particular instant in time, paying specific attention to the recently changed areas, obviously every team and ticket is different, but IMO unless the person doing the review is some sort of principal engineer mostly responsible for the code at large, this does not align with I would personally consider a cod…
I push through and do it anyhow, because this is one of those "they're not paying me to have an endless party" sort of things. But I completely 100% agree with the author that it is not a very useful view.
Earlier quoted context omitted.
I go back and forth on this. On the one hand, I really like constant deep feedback. I really like the consistency benefits of having another person say “that’s too much, I find that unreadable.” On the other, I have now been at a lot of places where it was very hard to get my code reviewed, latencies of days and sometimes weeks if folks are in a particularly heinous crunchtime... And then when it does get reviewed, t…
In this kind of context, I ask people what log level they'd like their review at. If you just want to get the code out the door, by all means, "error" or "warn" might be the right review depth, when you're confident in your code and don't want to be derailed with philosophy. If you're exploring a new concept and want all the ideas and brainstorming you can get in your feedback, "debug" log level is appropriate. Once…
It's a code review, not a QA session.
Earlier quoted context omitted.
Everyone committing to the main branch all the time sounds like a nightmare to me, to be honest. I would probably seriously consider quitting if this was forced on me. (Or rather, I would probably just run on my private git copy, and only pull every once in a while, and ignore that the main branch always changes.) When / how do you do code review in your suggested workflow?
You can have branches, but the ""rule"" is that no branch should live for more than, say, a day. This trades the integration complexity and problems for some new.. challenges :)
Earlier quoted context omitted.
This is called Continuous Integration, and its a shame that the term got nicked to now mean "that thing that builds our PRs somewhere". The idea was that everyone pushes to main all the time, which basically reduces integration time to 0, as everyone is doing it every couple of minutes on big teams. After a while you learn how to not step on people's toes (Introduce new classes incrementally, use docblocs documenting…
Everyone committing to the main branch all the time sounds like a nightmare to me, to be honest. I would probably seriously consider quitting if this was forced on me. (Or rather, I would probably just run on my private git copy, and only pull every once in a while, and ignore that the main branch always changes.) When / how do you do code review in your suggested workflow?
Earlier quoted context omitted.
> an organization that is set up to reduce code ownership, and they struggle to attract talent These might not be related though? > work is fulfilling > It exhilarating to start projects, then see them degrade inevitably into corporate monstrosities. What you describe does not sound pleasant. So it's not fulfilling after all?
Projects are fulfilling because they have public utility. The no individual code ownership policy is hard to bear for inquisitive minds, though. Thus the talent shortage.
I would not work on boring ass project with cripping management problems and think that's fulfilling. I assume you have recruiting problems because most people share that sentiment.
People like to work on interesting stuff without much office politics, that is "fulfilling" to them, even if outcome is app that would be "boring" to the outsider.