Join devRant
Do all the things like
++ or -- rants, post your own rants, comment on others' rants and build your customized dev avatar
Sign Up
Pipeless API
From the creators of devRant, Pipeless lets you power real-time personalized recommendations and activity feeds using a simple API
Learn More
Search - "merge request review"
-
*We colleagues were cursing Valentine's week*
Team Lead : Committed?
Me: No, I am single.
TL: *confused look* Did you committed that code?
Me: ohh yes! I raised the merge request as well.
TL: Ok. I will review it. *Moves away smiling inside*
Me: *looking at screen* *crying inside*6 -
[WARNING] THIS RANT IS NOT FOR HULKS OR SHE-HULKS
Here we fucking go again, currently, the time is 1:09 am in Malaysia, while I received a Pull request, so as a senior software engineer it is my duty to review the code before approving to merge develop branch. And this mother fucker decided to do this right after our CTO warned him about his coding style. Refer to https://devrant.com/rants/4699002/... for free cancer.
Our entire team is not happy working with this mother fucker, he is too arrogant.
Btw if he wants to insult me using codes, at least have the decency to draw some UML diagram , write proper documentation and write a proper logic, isn't better?62 -
Merge request
Title: Fix bugs
178 files, +615, -743
And it had passed review by not one, but two (allegedly) intelligent people.
HOW. THE. FUCK?!
Luckily I am overtaking that domain and won't allow such bullshit. Mainly because I will be the author of the commits.17 -
Just got a merge request to review. It's TERRIBLE
- 93 changes
- 23 commits
- includes multiple features
- includes new project configs
- includes refactoring
NO. Please don't do that.
Do smaller commits. One feature per MR. It will help you and the reviewer :D6 -
Indian web dev company (during the interview)::
We follow standards
Me:: Hey, can I get the project's github link so that I can fork it, do my tasks, run test cases, push and, make pull request that you can review, run integration test, and finally merge.
Indian web dev company:: What?? Here's the ftp credentials.
Me::12 -
The company I work at sends their developers out to other companies to help them work on projects and help them in other ways (advice when communicating to customers of on demand software for example).
While not on a project you are working in house training trainees and interns. Part of that is teaching them to show initiative and treating them as full developers. The 30 interns all discussed a git flow and code format.
During the third sprint (two weeks sprints) a team messaged me if I wanted to check their merge request for the sprint.
It took me a glance at the first file to know they didnt do any review themselves. I used my flywheel to check all their changes and without being able to read the code I saw indentation was all over the place, inconsistent bracket placements etc. I let them know I wouldnt check their code until it was according to their own standards.
Two days later I got the message to check it again. At first glance the indentation was fine so I started reading the code. Every single thing was hardcoded, not made to support mobile (or any resolution other than 1920×1080).
A week later they improved it and still not good. Gave them a few pointers like I would for any colleague and off they went to fix things. The code became worse and indentation was all over the place.
I told them the next time it shouldnt be a quick glance to be able to reject it again. By this time other teams came to me asking why it wasnt merged yet and I explained it to them. One of the teams couldnt do anything u til this was merged so I told them to implement it themselves. I was surprised that 4 teams came to me asking about a merge request, that was every team except the team whose pull request it was.
4 weeks after the intitial sprint the other team made a merge request and I had three small comments and then an hour later it was merged.
The other team messaged me why their merge request did went through (still havent seen any of their team in person, Im sitting 10 meters away from them behind a wall)
They also said that it was easier for them because they started from scratch. Thats when I called them in to discuss it all and if they were not interns but full time developers they would have been fired. I told them communication is key and that if you dont understand something you come in person to ask about it. They all knew I like teaching and have the patience to explain a single thing ten times, but the initiative should be theirs.
One of the team members is my current coworker and he learned his lesson by that. The others stopped with their study and started doing something completely else.
TL;DR
Merge request is open for 4 weeks, in the end another team started from scratch and finished it within a week. The original team didnt ask me questions or come to me in person, where other teams did.
DISCLAIMER: some of you might find it harsh, but in our experience it works the best for teaching and we know when people don't dare to ask questions and we help them in that too. It's all about the soft skills at our company.4 -
I was asked to fix a critical issue which had high visibility among the higher ups and were blocking QA from testing.
My dev lead (who was more like a dev manager) was having one of his insecure moments of “I need to get credit for helping fix this”, probably because he steals the oxygen from those who actually deserve to be alive and he knows he should be fired, slowly...over a BBQ.
For the next few days, I was bombarded with requests for status updates. Idea after idea of what I could do to fix the issue was hurled at me when all I needed was time to make the fix.
Dev Lead: “Dev X says he knows what the problem is and it’s a simple code fix and should be quick.” (Dev X is in the room as well)
Me: “Tell me, have you actually looked into the issue? Then you know that there are several race conditions causing this issue and the error only manifests itself during a Jenkins build and not locally. In order to know if you’ve fixed it, you have to run the Jenkins job each time which is a lengthy process.”
Dev X: “I don’t know how to access Jenkins.”
And so it continued. Just so you know, I’ve worked at controlling my anger over the years, usually triggered by asinine comments and decisions. I trained for many years with Buddhist monks atop remote mountain ranges, meditated for days under waterfalls, contemplated life in solitude as I crossed the desert, and spent many phone calls talking to Microsoft enterprise support while smiling.
But the next day, I lost my shit.
I had been working out quite a bit too so I could have probably flipped around ten large tables before I got tired. And I’m talking long tables you’d need two people to move.
For context, unresolved comments in our pull request process block the ability to merge. My code was ready and I had two other devs review and approve my code already, but my dev lead, who has never seen the code base, gave up trying to learn how to build the app, and hasn’t coded in years, decided to comment on my pull request that upper management has been waiting on and that he himself has been hounding me about.
Two stood out to me. I read them slowly.
“I think you should name this unit test better” (That unit test existed before my PR)
“This function was deleted and moved to this other file, just so people know”
A devil greeted me when I entered hell. He was quite understanding. It turns out he was also a dev.3 -
Code review, intern style:
Intern: Here is my pull request ...
Colleague: I see a problem with x, y, z. Could cause memory leaks.
Intern: Oh yeah you are correct, i'll fix that in the next one.
Intern: *merged* -
Dear team leader, If you tell me „I need to review this merge request before merging”, then make sure you are able to allocate time to do this.
If you need MORE THAN A WEEK to even start, then maybe your delegation skills are nonexistent. -
Day 1. Push branch. Make pull request😀
Day 5. Got reply to fix some silly stuff🤡
Day 6. Apply changes. Ask for review again.😊
Day 7. Needed to work for another project for two weeks🤝
Day 25. Came back and pull request and branch were deleted from server 🧐🧐
Day 26. I merge my local branch copy into master and push it to server. So long bitches! 💅💅6 -
Dev sent out a code review request.
I take about an hour, ask questions, make suggestions, general feedback, etc.
Today I noticed none of my questions were answered, developer closed the review, and the code merged into the production branch.
So I email him, asking him why the review was closed and why none of my concerns were addressed before merging to production.
Dev: "No one responded or left feedback, so I thought it was OK to merge up."
Me: "I reviewed and left feedback within the hour you sent the request."
Dev: "Oh yea...you did. Sorry. The code is already in production, but if you still want to leave feedback, create a work item, and I'll take a look."
No you won't.
An example of the code...The dev added an async method to a test harness *console app*. Why? .. check in comment was "Improves performance and enhances the developer experience.."
NO IT DOESN'T!
OK..that's off my chest. No one is getting punched in the face today.6 -
An amazing git story: A month ago coworkers did research and started with merge requests. Their workflow is as follows: A Feature is developed in a branch, then a merge request is opened. After a very short review it will be closed (rejected) and merged without request. (wtf!) After that the develop branch will be tested later, in case of bugs, a new branch per feature is opened.3
-
Last week,my friend asked me to fork and pull request on his github repository so that he could review my changes and merge them,I had included some more exciting features in his app but till now he has not merged my request,instead he just copied the changes I made in his repo into his repo manually and updated his project,still he has not merged my pull request!!
That guy is a genius!7 -
Don't need Netflix when you have a production deployment right before a long weekend. It has failed since last two weeks due to vulnerabilities present in one of libraries(P.S. FUCK JAVASCRIPT and Post release vulnerability scans!). You have rewritten the whole functionality from scratch twice! Security gates finally open for you, welcoming with arms wide open. So you click Deploy! DAFUQ!! FUCK MY LIFE! Deployment failed! It's only a 3 hour window to deploy! You frantically re-review your code, is it me?? Not again!! It isn't! Well, why is the deployment failing, you work against the clock. Going through configs, code, documentation! WTF is it?? Should I give up and raise a support ticket? Nope! You login to the server, sifting through logs and configs, there's a couple of other tickets with today's deadline. What are you going to do? And you get a hint! You take the hunch, change the config 5 minutes before deadline!
Get merge request approved, wait for the build, hit DEPLOY!! Nail biting 3 minutes! Your eyes fixed on the logs! Building..... Pushing instances..... Starting App..... SUCCESS!!! Finish the remaining tickets! Your long weekend still exists!3 -
I'm considering quitting a job I started a few weeks ago. I'll probably try to find other work first I suppose.
I'm UK based and this is the 6th programming/DevOps role I've had and I've never seen a team that is so utterly opposed to change. This is the largest company I've worked for in a full time capacity so someone please tell me if I'm going to see the same things at other companies of similar sizes (1000 employees). Or even tell me if I'm just being too opinionated and that I simply have different priorities than others I'm working with. The only upside so far is that at least 90% of the people I've been speaking to are very friendly and aren't outwardly toxic.
My first week, I explained during the daily stand up how I had been updating the readmes of a couple of code bases as I set them up locally, updated docker files to fix a few issues, made missing env files, and I didn't mention that I had also started a soon to be very long list of major problems in the code bases. 30 minutes later I get a call from the team lead saying he'd had complaints from another dev about the changes I'd spoke about making to their work. I was told to stash my changes for a few weeks at least and not to bother committing them.
Since then I've found out that even if I had wanted to, I wouldn't have been allowed to merge in my changes. Sprints are 2 weeks long, and are planned several sprints ahead. Trying to get any tickets planned in so far has been a brick wall, and it's clear management only cares about features.
Weirdly enough but not unsurprisingly I've heard loads of complaints about the slow turn around of the dev team to get out anything, be it bug fixes or features. It's weird because when I pointed out that there's currently no centralised logging or an error management platform like bugsnag, there was zero interest. I wrote a 4 page report on the benefits and how it would help the dev team to get away from fire fighting and these hidden issues they keep running into. But I was told that it would have to be planned for next year's work, as this year everything is already planned and there's no space in the budget for the roughly $20 a month a standard bugsnag plan would take.
The reason I even had time to write up such a report is because I get given work that takes 30 minutes and I'm seemingly expected to take several days to do it. I tried asking for more work at the start but I could tell the lead was busy and was frankly just annoyed that he was having to find me work within the narrow confines of what's planned for the sprint.
So I tried to keep busy with a load of code reviews and writing reports on road mapping out how we could improve various things. It's still not much to do though. And hey when I brought up actually implementing psr12 coding standards, there currently aren't any standards and the code bases even use a mix of spaces and tab indentation in the same file, I seemingly got a positive impression at the only senior developer meeting I've been to so far. However when I wrote up a confluence doc on setting up psr12 code sniffing in the various IDEs everyone uses, and mentioned it in a daily stand up, I once again got kickback and a talking to.
It's pretty clear that they'd like me to sit down, do my assigned work, and otherwise try to look busy. While continuing with their terrible practices.
After today I think I'll have to stop trying to do code reviews too as it's clear they don't actually want code to be reviewed. A junior dev who only started writing code last year had written probably the single worst pull request I've ever seen. However it's still a perfectly reasonable thing, they're junior and that's what code reviews are for. So I went through file by file and gently suggested a cleaner or safer way to achieve things, or in a couple of the worst cases I suggested that they bring up a refactor ticket to be made as the code base was trapping them in shocking practices. I'm talking html in strings being concatenated in a class. Database migrations that use hard coded IDs from production data. Database queries that again quote arbitrary production IDs. A mix of tabs and spaces in the same file. Indentation being way off. Etc, the list goes on.
Well of course I get massive kickback from that too, not just from the team lead who they complained to but the junior was incredibly rude and basically told me to shut up because this was how it was done in this code base. For the last 2 days it's been a bit of a back and forth of me at least trying to get the guy to fix the formatting issues, and my lead has messaged me multiple times asking if it can go through code review to QA yet. I don't know why they even bother with code reviews at this point.18 -
My best code review was when my merge request was accepted in less than a minute after creation. It was simple but I expected more time on review and accept action.
-
4 months ago, my team had the task of redesigning the login page of our main app. Really nice design. Since it was fairly simple, it was given to one of our summer camp guys to do something useful. After he finished, it was stuck on merge request and no one bothered to check it, as it was not important for our PO's, it simply got forgotten...
Last week, since I was bored and remembered about it, I decided to check it and fix the small issues it had, without telling anything to our PO, just did it, asked for code review and added it to our latest release.
Today I overheard 2 guys from analytics team:
"Hey, have you seen our new login page?"
"There is a new WordPress developer so he just does his job well"
Our application is not in WordPress, only our company's website is!
Our application is in Angular!
There is no new WordPress developer! We only have an offer looking for one!
WTF2 -
Picked up an issue to contribute to OSS for a community version of a major enterprise software. Did the changes, submitted a pull request. Someone reviewed it, asked for some changes, which i did and pushed the changes.
Then after some discussion with the guys working there, we thought of making some changes to the UI. Step in the company UI guy, he makes some changes, i merge his branch into mine and submit a new pull request.
Now, a new guy comes in to review the code, who has a problem with every change THEIR UI Guy did, and negates everything the first reviewer said, and asks me to do the changes, and boy was I pissed!!
But I did the changes, updated the PR, then the first reviewer comes in again, and suggests some more changes, most of them are for the code, THEIR UI Guydid!! Fucking psychopaths!! Never had i seen such paranoid people in my life!! Educate your fucking team first!!
I one again started with the changes but left mid way!! Now, even if i want to, will not update the PR!! FUCK YOU!!3 -
After three months of development, my first contribution to the client is going live on their servers in less than 12 hours. And let me say, I shall never again be doing that much programming in one go, because the last week and a half has been a nightmare... Where to begin...
So last Monday, my code passed to our testing servers, for QA to review and give its seal of approval. But the server was acting up and wouldn't let us do much, giving us tons of timeouts and other errors, so we reported it to the sysadmin and had to put off the testing.
Now that's all fine and dandy, but last Wednesday we had to prepare the release for 4 days of regression testing on our staging servers, which meant that by Wednesday night the code had to be greenlight by QA. Tuesday the sysadmin was unable to check the problem on our testing servers, so we had to wait to Wednesday.
Wednesday comes along, I'm patching a couple things I saw, and around lunch time we deploy to the testing servers. I launch our fancy new Postman tests which pass in local, and I get a bunch of errors. Partially my codes fault, partially the testing env manipulating server responses and systems failing.
Fifteen minutes before I leave work on the day we have to leave everything ready to pass to staging, I find another bug, which is not really something I can ignore. My typing skills go to work as I'm hammering line after line of code out, trying to get it finished so we can deploy and test when I get home. Done just in time to catch the bus home...
So I get home. Run the tests. Still a couple failures due to the bug I tried to resolve. We ask for an extension till the following morning, thus delaying our deployment to staging. Eight hours later, at 1AM, after working a full 8 hours before, I push my code and leave it ready for deployment the following morning. Finally, everything works and we can get our code up to staging. Tests had to be modified to accommodate the shitty testing environment, but I'm happy that we're finally done there.
Staging server shits itself for half a day, so we end up doing regression tests a full day late, without a change in date for our upload to production (yay...).
We get to staging, I run my tests, all green, all working, so happy. I keep on working on other stuff, and the day that we were slated to upload to production, my coworkers find that throughout the development (which included a huge migration), code was removed which should not have. Team panics. Everyone is reviewing my commits (over a hundred commits) trying to see what we're missing that is required (especially legal requirements). Upload to production is delayed one day because of this. Ended up being one class missing, and a couple lines of code, which is my bad (but seriously, not bad considering I'm a Junior who was handed this project as his first task at his first job).
I swear to God, from here on out, one feature per branch and merge request. Never again shall I let this happen. I don't even know why it was allowed to happen, it breaks our branch policies. But ohel... I will now personally oppose crap like this too...
Now if you'll excuse me... I'm going to be highly unproductive and rest, because I might start balding otherwise after these weeks... -
I’m the only junior software engineer at a small startup where I do mostly web development, as well as other bits and pieces (automation, ci/cd, etc)
Our software team is extremely small so we do not have anyone dedicated to QA. I usually just ask a team members with related experience to review my merge requests. So if I have a merge request for our ci/cd, I ask the software engineer with the most ci/cd experience to review the MR.
Recently I realized that my MRs will usually sit for days, and sometimes weeks without the reviewers taking a look. And when they eventually do, they don’t even run the code. It seems like they just gloss over it and look for obvious syntax or logic errors.
It makes me feel as if my code and efforts do not have much value to our team.
It also pisses me off because whenever a issue happens in our codebase, me and my code is the first thing blamed even if my code is not the issue
Is this typical in other companies? Or is this something I should speak to my boss about?4 -
Things that annoy me about my current place:
1 - Only 3 people out of a team of 12 developers are allowed to purge akamai, or merge pull requests to master on any of our 200+ websites. Apparently this is because us contractors are not allowed because the permanent employees have to be accountable for the code.
Despite this, no-one actually reads the code. You just throw up a request in the slack channel and boom, instantly 30 seconds later someone approves it, even if its 500+ lines of code.
2 - I've pushed for us to move to agile instead of waterfall, and got declined (which is fine), but the reasoning was that the dev team are not 'mature enough' to work that way. Half the devs here have 5+ years of experience so I don't get the problem here.
3 - There is zero code reviewing process in place. I just watched as a developer's 300 line merge request was approved within 8 seconds of it going live. No-one is allowed to comment on the code review or suggest changes as this would 'slow down development'. Within that 300 line merge request were tons of css being aimlessly commented out, and invalid javascript (introducing both bugs and security issues) that were totally ignored.
What is your thoughts on these above points?
Am I too narrow minded or is the development manager clueless?1 -
Nothing like code review and have to read the novel that is the comments on the merge request to understand what everyone's issue is with this one doc block. Wtf?