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 - "pull request review"
-
Simple 1 day task. This idiot takes two weeks and after 7 days of hounding finally opens a pull request.
I go in to review the code. Should be a simple 10-15 line patch.
13,000 lines of code changed.
THIRTEEN THOUSAND!
"I fixed a bunch of formatting mistakes and replaced all instances of single quotes to double. Consistency is important you know."20 -
I sometimes wonder my English is that bad ...
I reviewed a pull request. I commented on this line of code
a = getA() * -1;
saying that
"I think this logic should be in setA method instead."
When the guy asked me for the review again, the change was
- a = getA() * -1;
+ a = setA() * -1;
...2 -
[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 -
I joined a "multi-national" company in middle-east where 90% of the developers are Indian. And since it's a "multi-national" company with 50+ developers I thought they already figured it out. Most of them have 5-10 years of experience. They should know at least how to use git properly, deployment should be done via CI/CD. database changes should be run via migration script. Agile methodology, Code Review - Pull Request. Unit testing. Design Patterns, Clean Code Principle. etc etc
I thought I'm gonna learn new things here. I have never been so wrong in all my life...
Technical Manager doesn't even know what Pull Request is. They started developing the software 4 years ago but used Yii v1 instead which was released almost a decade ago. They combined it with a VueJS where in some files contains around 4000 lines of code. Some PHP functions contain 500+ of code. No proper indentions as well. The web console is bloody red with javascript errors. In short, it's the worst code I've seen so far.
No wonder why they keep receiving complaints from their 30+ clients.10 -
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 -
Why write any specs anymore? The juniors don't read them, nor do the product managers. I'm just talking to myself at this point.
So, I waste time writing these nice detailed tickets, then when I go to review the pull request, the whole pile of horseshit is half done, and when I ask the product managers for resolutions, they don't have a clue either.
So just give me the whole fucking app, I'll do the whole damn thing myself. See you on Mars in 2025 while you and your pleb asses serve me fries and burgers.1 -
A badass pull request review comment: 'A wise man can learn more from a foolish question than a fool can learn from a wise answer.'2
-
[ Introduction ]
In Internet culture, the 1% rule is a rule of thumb pertaining to participation in an internet community, stating that only 1% of the users of a website add content, while the other 99% of the participants only lurk.
[ The story ]
A year ago I had a problem with X software.
I opened a ticket on its repository but a week goes by and no one responds. I need it to work! So I opened a pull request and it got merged in a day or two after a quick review.
Seeing that the tickets were many and the maintainers were few, I decided to stay and help.
Today, I am in the top #10 contributors.
I have made 20 commits and edited 4k lines of code. (Honestly, it's not that much, at work I do way more than that, anyway...)
This repository is an alternative to another popular closed-source software and it's massively used by well-known companies
(tech-giants).
[ Stats ]
User base: 20.000 (all of them are devs)
Total contributors: 200 (1%)
Contributors with more than 1 commit: 60 (0.3%)
[ Consideration ]
I would never have believed a year ago that participation could be so low despite the number of dev-users being so high.
The software does not require great technical expertise and if you are using it for work then you already have the skill-set you need to contribute.
Now listen, I know that not everyone wants to contribute. I know right and I respect it ... but really:
The 0.3% ?! Only 60 devs on 20k are active contributors?! Only 200 (1%) devs have ever made a single commit and then they left.
Holy sh**11 -
Let me make a small change to this file, a one-line pull request will probably be accepted right away
* saves *
Prettier runs...
EsLint runs...
StyleLint runs...
60 changes submitted for review11 -
If you're going to request CRITICAL changes to thousands of records in the database, and approve it through testing which is done on an exact replica of production, then tell me it was done incorrectly after the fact it has been implemented and you didn't actually review the changes made to the data or business logic that you requested then you are an idiot. Our staging environment is there to ensure all the changes are accurate you useless human. Its the data you provided, I didn't just magically pull it from thin air to make yours and my job a pain the ass.undefined stupid data analysts this is why health insurance costs a buttload do your job fuckface idiots9
-
How do I even start?
The guy that's supposed to be our extra resource, our go-to person, asked me why node_modules and typescript output files are not committed.
Node.friggin.modules!
And by typescript output files, I mean the compiled .js files. Shoot me now...
All he does all day is waste time! Useless calls scheduled way too early, 'cause IST & why the heck not?
And don't even get me started on his "knowledgeable" colleague who spent 2 friggin' days on figuring out how to find an element in an array.
I mean ok, I get that the language is new and the syntax is different, but boy, how I wish that was the problem! But nooo, her issue was figuring out the damn logic behind it!
Not to mention that I gotta do the code review and she keeps ignoring the changes that I ask of her unless I raise that in our daily meeting and reports stuff as done even before submitting a damn pull request. Also, I gotta shut up and take it, 'cause they are the client's internal resources, which has me ranting about it at 2 a.m. T_T
Ugh...4 -
Today I found an error in how we handle credit on invoices in our software.
This is the first time my boss has ever made a legit pull request for me to review of his.
Damn I feel proud! -
Colleague: Hey could you review this diff?
Me: *opens Bitbucket*
Bitbucket: "Upgrade to Symfony 5", 19 files added, 732 modified, 14 removed
Me, 3 mouse scrolls and 20 seconds later: ✅3 -
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 -
Our project at work goes live in 3 weeks.
The code base has no automated tests, breaks very often, has never had any level of manual testing
will not be releasing with any form of enforced roles or permissions in our first release now due to no time to enforce, however there is a whole admin api where you can literally change anything in our database including roles.
We also have teams in various countries all working separately on the same solution using microservices with shared nuget packages and they aren't using them properly.
Our pull requests are so big - as much as, 75 file changes - in our fe app that I can't keep up with it and I honestly have no idea if it even works or not due to no automated tests and no time to manually test.
We have no testing team, or qa team of any sort.
Every request into the system has to hit a minimum of 3 different databases via 3 different microservices so 1 request = 4 requests with the load on the servers.
We don't use any file streams so everything is just shoved in the buffer on the server.
Most of the people working on the angular apps cba to learn angular, no one across 2 teams cba to learn git. We use git so they constantly face problems. The guy in charge has 0 experience in angular but makes me do things how he wants architecturally so half the patterns make no sense.
No one looks at the pull requests, they just click approve so they may as well push directly to master.
Unfinished work gets put in for pull request so we don't know if the app is in a release state since aall teams are working independently, but on the same code base.
I sat down and tested the app myself for an hour and found 25 fe only issues, and 5 breaking cross browser issues.
Most of our databases are not normalised. Most of our databases make no sense. 99% of our tables have no indexing since there is no expertise with free time to do it.
No one there understands css properly. Or javascript.
Our. Net core microservices all directly use ef in the controller actions so there is no shared code there.
Our customer facing fe app is not dry because no tests so it was decided it was better this way.
Management has no idea on code state, it seems team lead is lieing to them about things like having any level of tests.
Management hire devs that claim to be experts but then it turns out they have basically no knowledge of what they were hired to do, even don't know what json is or the framework or language they are hired for, but we just leave them to get on with it and again make prs too big to review.
Honestly I have no hope that this will go well now but I am morbidly curious to watch. I've never seen anything like the train wreck that we are about to get experience.5 -
"Can you review this pull request?"
Ok, sure
- Description in broken English
- HTML/CSS changes seemingly just for the fuck of it
- No user story listed OR
- User story listed has no description
- Mockup does not specify what should be changed
- Owner is offline because this entire team operates out of India
- Requirements said to exist but their location is unknown8 -
Had a guy put in a pull request with an empty file diff. He comes by two weeks later and asks when am I going to have time to review it.
-
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* -
developer makes a "missed-a-semicolon"-kind of mistake that brings your non-production infrastructure down.
manager goes crazy. rallies the whole team into a meeting to find "whom to hold accountable for this stupid mistake" ( read : whom should I blame? ).
spend 1-hour to investigate the problem. send out another developer to fix the problem.
... continue digging ...
( with every step in the software development lifecycle handbook; the only step missing was to pull the handbook itself out )
finds that the developer followed the development process well ( no hoops jumped ).
the error was missed during the code review because the reviewer didn't actually "review" the code, but reported that they had "reviewed and merged" the code
get asked why we're all spending time trying to fix a problem that occurred in a non-production environment. apparently, now it is about figuring out the root cause so that it doesn't happen in production.
we're ALL now staring at the SAME pull request. now the manager is suddenly more mad because the developer used brackets to indicate the pseudo-path where the change occurred.
"WHY WOULD YOU WASTE 30-SECONDS PUTTING ALL THOSE BRACES? YOU'RE ALREADY ON A BRANCH!"
PS : the reason I didn't quote any of the manager's words until the end was because they were screaming all along, so, I'd have to type in ALL CAPS-case. I'm a CAPS-case-hater by-default ( except for the singular use of "I" ( eye; indicating myself ) )
WTF? I mean, walk your temper off first ( I don't mean literally, right now; for now, consider it a figure of speech. I wish I could ask you to do it literally; but no, I'm not that much of a sadist just yet ). Then come back and decide what you actually want to be pissed about. Then think more; about whether you want to kill everyone else's productivity by rallying the entire team ( OK, I'm exaggerating, it's a small team of 4 people; excluding the manager ) to look at an issue that happened in a non-production environment.
At the end of the week, you're still going to come back and say we're behind schedule because we didn't get any work done.
Well, here's 4 hours of our time consumed away by you.
This manager also has a habit of saying, "getting on X's case". Even if it is a discussion ( and not a debate ). What is that supposed to mean? Did X commit such a grave crime that they need to be condemned to hell?
I miss my old organization where there was a strict no-blame policy. Their strategy was, "OK, we have an issue, let's fix it and move on."
I've gotten involved ( not caused it ) in even bigger issues ( like an almost-data-breach ) and nobody ever pointed a finger at another person.
Even though we all knew who caused the issue. Some even went beyond and defended the person. Like, "Them. No, that's not possible. They won't do such dumb mistakes. They're very thorough with their work."
No one even talked about the person behind their back either ( at least I wasn't involved in any such conversation ). Even later, after the whole issue had settled down. I don't think people brought it up later either ( though it was kind of a hush-hush need-to-know event )
Now I realize the other unsaid-advantage of the no-blame policy. You don't lose 4 hours of your so-called "quarantine productivity". We're already short on productivity. Please don't add anymore. 🙏11 -
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 -
Recently I've started thinking about how we are always told "No you can't do that" to everything. That feels like a theme in our industry.
I've also been thinking about how often people say well done to each other, or just comment that something is good in a pull request. Everything is always focused on bugs and mistakes - not good bits.
The first point conflicts with the idea that when using languages and frameworks you should follow their philosophy or you're gonna have a bad time - but in all other instances you mostly don't have wrong answers, just answers that can be better so a lot of stuff is opinion based.
I've decided to change my ways and focus just as much on good stuff as bad when I review code and to make sure I'm focusing just as much, if not more, when people do something good.
I think I do a good job, but I don't think I've been told I'm doing a good job or that anything specific is good more than a couple times in the last year - mostly in mandatory reviews. What about you?2 -
i was hired to join a team of old devs (40+) in an unnamed European country "yay goodbye 3rd world it's time to enjoy the quality of life" assist with enhancing already existing software and creating new solutions.
prior to my arrival most things were slow and super buggy, looking at the code base it shouldn't be a surprise, amateur hour everyone, logic implemented that is not needed, comment driven development, last time code review was done back in 1996. lots of anti patterns.
i swear there is a for loop that does nothing but it loops through a 100+ elements list, trunk based development with tfs since git is "not really needed"
test projects are not there.
>enter me an educated fool, with genuine passion for the craft and somehow a decent amount of knowledge.
>spent the last year fixing stuff educating people on principles and qualities.
> countless hours of training and explaining. team is showing cooperation, a new requirement comes in to develop with react.
> tear my ass creating reusable shit and self explanatory code with proper naming etc using git with feature branching, monday is first deployment day.
> today a colleague was working on an item submit a pull request and self approve it
> look at the code..... WTF the dumb fuck copied and pasted the whole code from different kendo components but somehow managed to refractor the name to test component, commented out all the code that he didn't use did the api call directly from the component, has 2 useeffects that depends on the a fucking text box changes for no reason, no redux implementation, the acceptance criteria is not achieved, and it doesn't work it just look right.
> first world country shit cannot scold, cannot complain, lead by example.
>asked him why you did this, the response was yeah probably i shouldn't have done that, i really didn't understand anything in the training but didn't want to waste time!!!!
> rest of the team created a different styled disaster with different flavors they don't even name their shit the same way.
fellow developers I'm stuck in a spaceship with a bunch of imposters, seriously i never cried in my entire life now I'm teary and on the verge of a break down.
talk with management "improving needs time" and offers me to join a yoga session to release the stress as if reaching nirvana would deliver shit on monday.
i really don't know what do is this a rant, is this a cry for help, I'm not sure, any advice is welcomed.7 -
Do not, I repeat... DO NOT approve a pull request that you cannot adequately review. You're more to blame than the person who wrote the code in the first place when it fails.
-
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 -
So I did a code review for a colleagues pull request and I've noticed that he hasn't written the PHPDocs for a lot of the classes and functions. One minor thing I wrote is to add the author for the class.
About 2 mins after writing that comment he came over to tell me why should he write the author in the comments when people can just go look at the git commit logs. I was like WTF? I asked why would he do that, his answer was that if there's an issue, we can just use git blame to identify the author. To me that makes no sense as git blame isn't supposed to be used like that.
It's guys like these are the ones who don't document anything whether in an online document or even in code. And they just make work harder for the rest of us.2 -
Not to burn professional bridges every time I have to review a pull request, not the biggest challenge but the one I face more often.
-
We've recently employed a new lead dev that seems to have a problem in that his solution is always the correct solution.
On a typical day, whenever I push code up for review via pull request, every single ticket I work on, he has something that has to change which doubles the amount of time of each ticket.
I'd be fine with this if the other 2 developers also think he's a bit of a headache in terms of his opinion but a lot of the time, there is always.. ALWAYS something that has to change because his method is better than mine.
For example, just now I pushed up some code that literally just adds in the user's email to the view which is already in the store for that action/effect anyway. I added one line of HTML.
He comments saying that I need to change the way it gets the email by doing a different request in the effect, to get the current user id, and from that match it against the email address, and THEN display it in the view.
This ticket took me 5 minutes. He's making me make it 30-60 minutes (to understand his requirement and implement it).
Is this normal? Am I over reacting?
Opinions please!7 -
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 -
Finally pushed all review changes to that GitHub Pull Request of mine that had been open for months!
Hopefully it will get reviewed soon with possibly very minimal to 0 changes and then merged!!! :D1 -
How do you guys deal with reviewing garbage code?
Do you waste a ton of time, trying to understand the code, so you can write constructive feedback?
Today I wasted 4 hours reviewing something like that and I'm still not finished.10 -
So I fix a bug and I create a PR, someone reviews it and leaves a couple of comments, I address those comments and push up my updated code thinking “great I should be ok to move onto this big story waiting for me”
Then some Expletive.random(); from a totally different team who has no context of my change comes in and starts leaving petty comments. He literally pointed out 3 different things that could be made private/package-private.
Bugger off and focus on your own team’s work instead of leaving comments about relatively trivial things on my PRs.
Apparently he is well known for this. I can tell we are gonna have some fun encounters...1 -
So, I fixed this shitty code, real shitty, inlined some shit and shaved off some other shit and it was fixed...
The reviewer says we'd better request a review from the dude that's actually responsible for that piece of shit code - what the fuck...
Here comes this bossy fucker saying they don't really understand that shit so they don't know for sure if there could be a better way...
Me, ignorant as always, popped a vessel figuring out a cleaner way.
I tell those sick fucks that we'd need to change some shit over at another repository, also maintained by the latter turd.
The latter turd says they like my second suggestion better, to which I reply,
'ok, I agree.'
In my mind that pull was done, should be closed and water under the bridge but oh how clueless I was...
SIX FUCKING MONTHS later the same shitbag pops out of god knows where asking if I still wanted to work on the pull....
"Motherfucker, my pull was for this fucking code, not for doing work on the other, obviously I'm not interested in doing that or else I would've opened a pull there instead of here, dumb-dumb" - I thought
Thou what I said was:
"No, I don't. I agree it's a problem better solved at the other repository."
Maybe I was a bit mean, was I? I don't know, honestly, people confuse me2 -
I tried to get this trainwreck of a middleware running again. Generations of imbecile devs turned this once ambitious project into a dumpster fire. I had some feeble hopes. Wrote a little framework to get this braindead architecture back on its track. BUT EVERY PULL REQUEST I GET TO REVIEW SHOWS THEY DON'T CARE. FML! Why am I even trying. I should have known these morons just want to pour gasoline onto an already blazingly burning software. GODDAMMIT SHOW SOME ENGINEERING SPIRIT. Why are you even in this industry in the first place?1
-
Genuine question, what was the most comments you've left in a single code review?
Just reviewed pull request submitted by a developer working for a contractor company and needed to leave 70 comments. Seventy.
Opened LinkedIn and saw a post from that same developer saying he left the contracting company an hour ago. I still can't believe it.14 -
When Monday comes round and all you can think about traveling to work is the giant pull request you were asked to review on Friday 🙈
-
How the fuck am I supposed to fucking keep working if these fucking clowns add mandatory peer code review and passing build gating on main repositories (which I completely agree with to be fair) but they don't fucking review pull requests at all? For fuck's sake, am I the only one that reviews them seriously and promptly in this shit ass fuck company? I follow all the recommended guidelines so don't bullshit me with "iT iS nOt FuN tO rEvIeW pUlL rEqUeStS", do your job or just remove yourself from the fucking gating process, you worthless admin ass crust.
And don't get me started on fucking builds that fail randomly because some worthless shit bucket added unstable networking tests as unittests somehow, making your pull request get auto-disapproved by peers upon failure.
I got so many pending pull requests and management won't do fuck all about it because they won't force people to do their job by fear of pushing them around and get HR complaints that I am tempted to simply give up and just start playing videogames.5 -
!Rant
Tldr: great spike to solve deployment problem may be a wasted effort.
Deployments of an ancient electron application need to be done in CodeDeploy to deploy the latest build. Customer hour restrictions cause this to be done only after midnight, and manually checked.
The whole team knows this is the wrong method of deployment and that there are many other operational problems with the project.
A few other senior team members get together and decide to spike out a way to use electron auto-deployment to accomplish this without using code-deploy at all.
After a shallow dive into this subject, we all get pulled aside to handle a change in another part of the software ecosystem. It happens. We leave the spike behind.
A junior-intermediate developer on the team pics the project up and gets a good spike going in a day and a half! We are all high fives and beers. This is Friday.
By Monday there is a pull request in for code review and it looks solid. Seems like it will make deployments a lot better.
Preparing the last deployment (hopefully) with CodeDeploy ever...
Marketing team members inform us that they are running an add system on the customer devices and to do it they are using Linux.
The current application being deployed is using Windows 10 (yeah, another problem).
They say they have made plans to move our application over to Linux. This means we may not be able to launch the junior devs great spike and the old deployment method may stay for the time being.
Meetings soon to find out how all of this will hash out.
End of rant. I hope I'm doing this right -
Oh you have a pull request that fixes a bug impacting customers atm? This is a perfect opportunity to write a novel that debates optimal coding practices in the review
Smiling eye twitch2 -
Writing documentation is one of those tasks that most developers don't like doing. Especially when it comes to writing in say a Word/PDF file, an online wiki, or Confluence. It's time consuming and a pain in the ass.
But even if you don't like it, at least write comments in your source code! I hate having to keep writing "Write the PHPDocs for this class/function" in every pull request that I review. It's wasting my time writing such comments when it's such a basic thing to do when writing source code.31 -
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 -
Dude i asked you to review my pull request because i thought we were cool. The code change is about a simple rename, SO FUCK OFF WITH THE REFACTOR SUGGESTIONS. STOP DOING THAT TO MY CODE REVIEWS2
-
1) Read the ticket.
2) Create a branch with ticket number in name.
3) Move ticket to Working now section.
4) Make some changes according to the ticket.
5) Commit changes to branch. Than pull it.
6) Create pull request and submit it.
7) Move ticket into In review section.
8) Move to another ticket.
Tickets:
#7 - Change background size in product item.
#8 - Add icon to info flash message.
#9 - Add adaptiveHeight parameter to the slick slider.
Done, now another 30 tickets...
Yep, this is my workflow i'm forced to now.2 -
Made a pull request..review tonight... Deployment tomorrow night.. or morning according to new york time... Hope I don't wake up to pissed off comments in the request.
-
Writing my first code review. Even though it really is a nice review and I'm happy with the solution code, I still somehow feel like an asshole for each critique I make. Maybe it's unavoidable with code reviews / pull requests?3
-
So sick of my coworker explaining to me how I should do a task. Dude, I've already planned out how I'm going to handle the situation. If you really want to help, wait until I create my pull request, review it, and then make your suggestions there. Unless I ask for your advice on how to do something, I don't need you to tell me how you would do things, especially since i have, what, 5 years working in the framework when you have 2 months?
-
Any android devs here? I created a simple interview tech assignment app with 2 screens (item grid and item details) with latest architecture (Compose, Retrofit, Coil, Room). Looking for a review or any kind of feedback/ideas regarding what needs to be fixed/refactored. Repo is here: https://github.com/appdevv/DemoApp
If you want, feel free to pull the repo and just make a pull request with your comments.5 -
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 -
That moment when you have to send a 3 line pull request for one of the newer guys to review immediately, and he gets super anal retentive over the order in which arguments appear2
-
This team mate of mine always finds a way to escape my suggestions in his pull requests.
Today he told me that what I'm asking him is just my personal preference and that it's not a thing just because our formatter isn't powerful enough to do it automatically.3 -
People that approve pull requests without looking at them!
No tests or so bad they would do more use by not existing, typos, the code follows none of the design practices and the code obviously will not compile and thereby breaks builds in trunk for everyone.
Because of course they only asked one person to review it and then merged it immediately. -
I don't get it, why having more than one decorator looks "weird"? that's idiomatic Python AFAIK, that's sounds more like nitpicking on aesthetics :/1
-
Is this a justified code review comment or a bully?
Code reviews are weakness of this industry which has the potential to attract bullies. Abuse of the comment box in a pull request and bombarding the employee with hundreds of comments can cause stress, frustration, burnout and finally resignation and costs of fulfillment for the organization. While companies should find and stop bullying in the work place, what kind of code review comment is considered a bully and why? Any of below traits can mean you are dealing with a bully:
1. Claims the code needs to be changed but doesn't say how. So no matter how many times you change your code, he can repeat the same comment: "Your code is still bad due to blah blah and it needs to be changed".
2. Provides how the code should be changed, but the change doesn't add up to quality, security, performance, readability, etc. i.e. "Why did you use a for loop here? Use a while loop instead". Or "Why did you write it using three classes A, B and C? Instead write it using 4 classes D, E, F and G which does blah blah". In the later case, not following the review comment, you won't get approval. Following the comment means you need to rewrite your whole code. After which, you might again receive more comments to change other parts of your code!
3. Claims the requested change is due to standards but claimed standard does exist anywhere. Internet, company wiki, university course books, anywhere. In more severe cases of psychopathy, the bullying person refers you to a link which hours later turned out to be written by himself! Have fun describing what has happened to your manager or team leader... .
4. Asks the code to be changed in a way that supposedly is closer to standard or of better quality, security, performance, etc. But the proposed way will not work and is the main reason you didn't do that in the first place. So you start arguing forever in the comment box over why his method won't work!
If you cannot see any of the above traits, then keep calm, take a breath, fix your code. Otherwise you might be victim of a bully.3 -
When you open all the bitbucket emails you have to process after you spent just a half day doing your own tickets.
-
Today I submitted to code review the first iteration of a microservice done with Ramda and flow by request of my collegues. This is the first time they look at anything similar to functional code or typed js, and only one of them took the time to actually do a review.
I really like having my code reviewed and reviewing others', but please don't pester me to make a PR for a microservice you'll never look only to bail off as soon as you see something new that scares you. Buckle up and learn new stuff!