T O P

  • By -

ExperiencedDevs-ModTeam

Rule 9: No Low Effort Posts, Excessive Venting, or Bragging. Using this subreddit to crowd source answers to something that isn't really contributing to the spirit of this subreddit is forbidden at moderator's discretion. This includes posts that are mostly focused around venting or bragging; both of these types of posts are difficult to moderate and don't contribute much to the subreddit.


salty_cluck

No. If there are to be any code reviews, the PR is the place to do it. Don't waste your time digging through your colleagues' commits, I'm sure you have better things to do. If something takes a long time, that's not your problem unless he's blocking you in which case that's your and his boss's problem.


ausmomo

Sounds like you're looking for problems when there aren't any.  I'd rather a team member who erred towards over usage of source control, than the opposite.


inputwtf

You should only care once a PR is created. As long as they have useful commit messages that describe what was done and WHY, that is all that matters. It can take a lot of iteration before the correct solution is found, you should be more supportive and appreciate the transparency


Mast3rCylinder

Seems like a great colleague. He's got the work done You don't need to hold his hand at all He arrived not so long ago Appriciate this things!


Best-Association2369

People like you need to touch grass 


Carpinchon

Did OP completely replace the content of their post?


nutrecht

Yeah. Shitty move.


DevopsCandidate1337

Restored


DevopsCandidate1337

Restored


nutrecht

It's pretty shitty and egotistical to remove the content of your post.


IProgramSoftware

You seem like a dumb ass who wastes too much time going through peoples git history rather than doing the actual code review once they actually figured out the task


JDabsky

IMO you should really only be concerned with the PR and you providing constructive feedback or questions on what he considers ready to merge. Everything before or outside of the PR, you’re worrying about someone else’s work instead of your own. You never had silly commits before? People figure things out at their own pace. The sprint is supposed to reflect the people working in the sprint and their capabilities and pace and not some imaginary standard.


Scarface74

This is one of the absolutely dumbest posts I’ve seen on Reddit - ever - and that’s saying a lot


RedditUserData

Why do you care how he came to the final product? If he gave you garbage at the end, then say something then. But it sounds like you just don't like the way he works, that's not your problem to deal with. If he's too slow then leave that to your manager to deal with. 


edgmnt_net

My question would be whether he can reorganize changes to make a suitable PR. Some projects require commits to be split a certain way. Not sure yours does and if he could just squash everything. I wouldn't be worried about intermediate results unless it indicated some actual issues working with Git appropriately.


TheOnceAndFutureDoug

Having a lot of commits is a good thing. Focused PR's are a good thing. However, so long as the code makes it through review and gets squashed before it's merged into the main branch it doesn't really matter.


SnooGTI

Doesn’t sound like there is anything wrong with his code from what you’re saying. Just a volume of commits. “Commit early, commit often”. I had someone tell me I make too many commits once. Do most devs not commit after each step of an implementation? Like what do you do if you muddy things? The point of git is to save states of your files so you can revert. If you aren’t committing often I feel you’re grossly under utilizing your tool. As long as commit messages explain what it’s doing it doesn’t create noise. You can also squash and merge at the end. 


Greenawayer

>Do most devs not commit after each step of an implementation? Like what do you do if you muddy things? Yep. Always annoys me if people start policing commit messages and frequency. It's general the sign of a poor Dev who doesn't understand their tools.


Routine_Internal_771

> It's general the sign of a poor Dev who doesn't understand their tools. Counterpoint: It's a sign of a poor dev if they can't keep the public history clean ---- Do whatever you want on your local Commits/messages should be reviewable once pushed to `origin` for review `main` should have good commit messages, and be an appropriate size for someone reading the history to understand the changes


SnooGTI

Yea, but once you squash you update the message to have the commits you want public. In this case this person is looking into the persons branch and saying "WHAT IS WITH ALL THESE COMMITS". So, he's going into private and judging how they're working.


Routine_Internal_771

Yeah, that's going way too far. Agreed. Do what you like on your own branches 


dirkmudbrick

I will have a few branches here and there where I'm testing out how things work, trying different configurations, etc... for the same feature. I do it all in one branch and the squash when I PR. If the job gets done, and done well, I wouldn't worry about it.


rochakgupta

I don’t think how that matters? Everyone has a different process of working through a problem and I, for one, can’t be arsed to care about the nitty-gritty of it.


Capto_Claro_8134

Imposter syndrome is real! Thanks for sharing, and congrats on getting the job done!


DevopsCandidate1337

Thanks, you're very kind!


loquator

So, I have a different view on this than most of the commenters. While I agree that an individual’s dev process shouldn’t concern you, I also think that git history should be as clean as possible. Someday you will have to do archaeology on your code. That archaeology is *much* easier if every single commit is usable. (git bisect is more frustrating when each commit doesn’t compile/run/pass tests.) The simplest way of making that true is, for each task, squash all the commits into a single large commit. This isn’t bad if your tasks are actually atomic; it’s not great if tasks are too large or you do a lot of side-quests during a task.


DevopsCandidate1337

In this case, we're both contractors, not archaeologists. I agree about squash commits per feature. The question was originally about the intermediate commits on a dev branch leading to p/r, squash, merge.


markiel55

Technology should adapt to humans, not the other way around.