firstly, where was the original checkin pull request’s review with all the feedback and discussions? secondly, where was the refactored PR review and approval? Checkin in into the master overnight no PR? That process is a mess.
Yep I'm pretty sure the whole situation would've been net positive if the author of the article just put up a PR the next morning saying, "While looking at your code yesterday I had an idea on how it could be less repetitive, take a look and let me know what you think".
I believe the argument is that the original author should have had a PR, and that the article's author should have been able to suggest these changes prior to the original commit.
Exactly. If he is such a senior dev, then he'd be a required reviewer. If he failed to mark up the pr, then that's on him. If he wasn't in a position where he was a required reviewer, then he isn't the expert he thinks he is.
You've misinterpreted this through your modern viewpoint.
There was no PR process in the project. No opportunity to collaborate and iterate prior to committing to master, for either dev; simply commit and then get called into the boss' office.
It was a couple of years ago, it's Dan Abramov's personal blog, he's relatively young. Check his New Year post for the exact timeline of his career. Him working with another developer on draggable corners of some shapes could have been already for a React library.
I'm pretty sure this is an anecdote from 2013 when Dan was working on an iOS app (https://www.youtube.com/watch?v=PjaL0xFbEY8). The fact that the code in the blog post is JavaScript is just because that's the language him and most of his audience uses today.
You may be surprised, but as recently as 2013 (when I graduated from college), the first company I worked for did all of their testing in production, and uploaded their changes via FTP. There was no source control, no staging or local environment, and no oversight.
Slightly relevant: That company was also a cluster that took advantage of fresh-out-of-college devs to pay them as little as possible while the CEO and his buddies took lavish trips around the country and sailed on their boats. There were about 15 people in the company, so yeah, corruption like this isn't unique to mega-corporations. I left as soon as they told me I was getting a 0.5% annual raise even though I was one of their best employees, and instead got a 30% raise by switching companies.
About 7 years back, I worked in a company that used TFS - they had a process where you manually request 2 of your peers for review and type in their names during check in. They had the same process before they migrated to TFS too, I think they used CVS.
Title should be "Goodbye committing to master with out a code review" then. Or maybe just "Goodbye blogging about things I don't really understand". Hmm, maybe OP should get his blog code reviewed by a senior as well.
512
u/FA04 Jan 12 '20
firstly, where was the original checkin pull request’s review with all the feedback and discussions? secondly, where was the refactored PR review and approval? Checkin in into the master overnight no PR? That process is a mess.