Manager: Hey how come you left so many comments on my PR?

Dev: Well you’ve just recently learned how to code so there’s going to be a lot of things to learn beyond what you’ve picked up in your online coding tutorials. Don’t worry it’s only minor things like you put everything all in one function, left outdated comments in the code, have if statements 4 levels deep, have a console.log after every line of code some of which log .env variables, skipped error handling, cast to “any” a bunch instead of using more specific types, didn’t write any tests and some unrelated tests are now failing due to a circular dependancy.

Manager: THAT IS SO DISRESPECTFUL!!APPROVE MY PR IMMEDIATELY. IT WASN’T EVEN EASY FOR ME TO CREATE THE PR, NOW I HAVE TO MAKE AN UPDATE!? YOU’RE THE DEV, YOU SHOULD FIX IT NOT ME!! NEVER COMMENT ON ANY OF MY PRS AGAIN.

I found a cool project on GitHub. I forked it and added a simple dev server with the intent of making it more accessible which could lead to more activity = improved project. I created a PR with small concise commits with very informative messages.

The guy who owns the project comments and says "I don't want your dev server, I have an apache instance locally on my computer". I tell him "Ok sure, but wouldn't it be nice if everyone else also had a nice dev server which can be started with a single command?", and other people join the PR and agree with me that we should make it available for everyone.

But the fucking idiot doesn't care, "No, I prefer to use my apache server". YOU FUCKING ASS WIPE, why do you even put it up on GitHub if you don't want contributions to make your project better and more available? I saw other open PRs where he basically did the same thing, left a snarky comment without merging it. What a fucking tool. Worst spent time ever.

FUCK YOU

Code review time:

Hey Rudy, can you approve my PR? ??? Shouldn't it be can you review my PR?(thought to myself)
Anyway, as a new practice, we(royal we) do not approve PRs with js files. If we touch one, we convert it to typescript as part of a ramp up to a migration that never seems to get here. But I digress.
I look at the laziest conversion in history.

Looked like
Import 'something';
Import 'somethingElse';
Import 'anotherSomething';

export class SomeClass {
public prop1: any;
public prop2: any;
public prop3: any;

public doWork(param: any){
let someValue = param;
// you get the idea
return someValue;
}
}

Anyway, I question if all the properties need to be visible outside of the class since everything was public.
Then if the dev could go and use type safety.
Then asked why not define the return type for the method since it would make it easier for others to consume.
Since parts of the app are still in js, I asked that they check that that the value passed in was valid(no compilation error, obviously).
Also to use = () => {} to make sure "this" is really this.
I also pointed out the import problem, but anyway.

I then see the his team leader approve the PR and then tell me that I'm being too hard on his devs. ????

Do we need to finish every PR comment with "pretty please" now?
These are grown men and women, and yet, it feels oddly like kindergarten.

I've written code in the past that wasn't pretty and I received "WTF?" as a PR comment. I then realized I ate sh*t on that line of code, corrected it and pushed the code. Then we went to Starbucks.
I'm not that old(35), but these young devs need to learn that COMPILERS DO NOT CARE ABOUT YOUR FEELINGS!!!!!
Ahhhhhh
Much better.
Thanks for the platform.

New developers(5-6 years experience) these days are so pathetic. They dont have any sense of code review. All they want is to put their opinion out without giving any thought.

I had a PR for review today which contains mock specification to match a regular expression and return the corresponding response

The regular expression I put was

104000(02|06|20|48)

Now, this guy comes and puts a comment that we could "simplify" as 104000\d{2}

I replied, the ending digits are not contiguous. The specific pair of digits have to match for these mocks.

Then this guy replied, then we could simplify as 104(0{4}(2|6)l0{3}(20|48)).

I said, I cannot understand how that is simplification. Why do we need such a complex regex to match something very straight forward.

And the guy replied, we should be writing proper regexes, otherwise we could just specify everything explicitly.

I was like WTF man. You try deciphering this next week without taking at least a minute to know which values are matched.

Anyhow, another senior person approved my PR, and I merged it.

Today marks the first day i needed to use a fork of a Github repo, because it just didn't contain a very basic feature i needed. The PR which fixes it has been open for months without a comment on why it's not merged.

I mean i'm glad i found a solution at the end, but goddamn it's so sad when a popular repo is not maintained well.

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.

After noticing 4 operations in a single line, I comment a pr stating the line is not simple to read as there are many operations which can be, especially in the eyes of a junior, hard to read.
I proceed to suggest a better solution.

Colleague: “what??? How is this not readable??? Is it [op1], [op2], [op3] or [op4]? 🤷‍♂️”

I kindly explain this person that it’s not about the single operations, but the fact they are all on a line. Inside an object assignment.

Colleague: “you should learn stuff! (4 links to websites giving that snippet of code”

Ah yes, the oldie: “but other people are jumping off a cliff, why don’t WE do it???”

Github 101 (many of these things pertain to other places, but Github is what I'll focus on)

- Even the best still get their shit closed - PRs, issues, whatever. It's a part of the process; learn from it and move on.

- Not every maintainer is nice. Not every maintainer wants X feature. Not every maintainer will give you the time of day. You will never change this, so don't take it personally.

- Asking questions is okay. The trackers aren't just for bug reports/feature requests/PRs. Some maintainers will point you toward StackOverflow but that's usually code for "I don't have time to help you", not "you did something wrong".

- If you open an issue (or ask a question) and it receives a response and then it's closed, don't be upset - that's just how that works. An open issue means something actionable can still happen. If your question has been answered or issue has been resolved, the issue being closed helps maintainers keep things un-cluttered. It's not a middle finger to the face.

- Further, on especially noisy or popular repositories, locking the issue might happen when it's closed. Again, while it might feel like it, it's not a middle finger. It just prevents certain types of wrongdoing from the less... courteous or common-sense-having users.

- Never assume anything about who you're talking to, ever. Even recently, I made this mistake when correcting someone about calling what I thought was "powerpc" just "power". I told them "hey, it's called powerpc by the way" and they (kindly) let me know it's "power" and why, and also that they're on the Power team. Needless to say, they had the authority in that situation. Some people aren't as nice, but the best way to avoid heated discussion is....

- ... don't assume malice. Often I've come across what I perceived to be a rude or pushy comment. Sometimes, it feels as though the person is demanding something. As a native English speaker, I naturally tried to read between the lines as English speakers love to tuck away hidden meanings and emotions into finely crafted sentences. However, in many cases, it turns out that the other person didn't speak English well enough at all and that the easiest and most accurate way for them to convey something was bluntly and directly in English (since, of course, that's the easiest way). Cultures differ, priorities differ, patience tolerances differ. We're all people after all - so don't assume someone is being mean or is trying to start a fight. Insinuating such might actually make things worse.

- Please, PLEASE, search issues first before you open a new one. Explaining why one of my packages will not be re-written as an ESM module is almost muscle memory at this point.

- If you put in the effort, so will I (as a maintainer). Oftentimes, when you're opening an issue on a repository, the owner hasn't looked at the code in a while. If you give them a lot of hints as to how to solve a problem or answer your question, you're going to make them super, duper happy. Provide stack traces, reproduction cases, links to the source code - even open a PR if you can. I can respond to issues and approve PRs from anywhere, but can't always investigate an issue on a computer as readily. This is especially true when filing bugs - if you don't help me solve it, it simply won't be solved.

- [warning: controversial] Emojis dillute your content. It's not often I see it, but sometimes I see someone use emojis every few words to "accent" the word before it. It's annoying, counterproductive, and makes you look like an idiot. It also makes me want to help you way less.

- Github's code search is awful. If you're really looking for something, clone (--depth=1) the repository into /tmp or something and [rip]grep it yourself. Believe me, it will save you time looking for things that clearly exist but don't show up in the search results (or is buried behind an ocean of test files).

- Thanking a maintainer goes a very long way in making connections, especially when you're interacting somewhat heavily with a repository. It almost never happens and having talked with several very famous OSSers about this in the past it really makes our week when it happens. If you ever feel as though you're being noisy or anxious about interacting with a repository, remember that ending your comment with a quick "btw thanks for a cool repo, it's really helpful" always sets things off on a Good Note.

- If you open an issue or a PR, don't close it if it doesn't receive attention. It's really annoying, causes ambiguity in licensing, and doesn't solve anything. It also makes you look overdramatic. OSS is by and large supported by peoples' free time. Life gets in the way a LOT, especially right now, so it's not unusual for an issue (or even a PR) to go untouched for a few weeks, months, or (in some cases) a year or so. If it's urgent, fork :)

I'll leave it at that. I hear about a lot of people too anxious to contribute or interact on Github, but it really isn't so bad!

PR reviews, people leave great comment, some suggestions and there could be a discussion.

But please don't be the person who goes there and add those weird and useless comments such: "CI failed, your PR is behind dev branch..."
That's the role of the provider(Github, Bitbucket.....) and adding that comment is just a waste of time.

This start to happen more often and I had to rant about it :D

One of the best feelings is when you're reviewing PRs, leave a comment of an issue you're seeing & another developer supports & agrees your comment, so the original creator of the PR doesn't think you're making things up.

A bug is born
... and it's sneaky and slimy. Mr. Senior-been-doing-it-for-ears commits some half-assed shitty code, blames failed tests on availability of CI licenses. I decided to check what's causing this shit nevertheless, turns out he forgot to flag parts of the code consistently using his new compiler defines, and some parts would get compiled while others needed wouldn't .. Not a big deal, we all make mistakes, but he rushes to Teams chat directing a message to me (after some earlier non-sensible argument about merits of cherry picking vs re-base):
Now all tests pass, except ones that need CI license. The PR is done, you can use your preferred way to take my changes.

So after I spot those missing checks causing the tests to fail, as well as another bug in yet another test case, and yet another disastrous memory related bug, which weren't detected by the tests of course .. I ponder my options .. especially based on our history .. if I say anything he will get offended, or at best the PR will get delayed while he is in denial arguing back even longer and dependent tasks will get delayed and the rest of the team will be forced to watch this show in agony, he also just created a bottleneck putting so many things at stake in one PR ..

I am in a pickle here .. should I just put review comments and risk opening a can of worms, or should I just mention the very obvious bugs, or even should I do nothing .. I end up reaching for the PM and explained the situation. In complete denial, he still believes it's a license problem and goes on ranting about how another project suffering the same fate .. bla bla bla chipset ... bla bla bla project .. bla bla bla back in whatever team .. then only when I started telling him:
These issues are even spotted by "Bob" earlier, since for some reason you just dismissed whatever I just said ..
("Bob" is another more sane senior developer in the team, and speaks the same language as the PM)
Only now I get his attention! He then starts going through the issues with me (for some reason he thinks he is technical enough to get them) .. He now to some extent believes the first few obvious bugs .. now the more disastrous bug he is having really hard time wrapping his head around it .. Then the desperate I became, I suggest let's just get this PR merged for the sake of the other tasks after may be fixing the obvious issues and meanwhile we create another task to fix the bug later .. here he chips in:
You know what, that memory bug seems like a corner case, if it won't cause issues down the road after merging let's see if we need even to open an internal fix or defect for it later. Only customers can report bugs.

I am in awe how low the bar can get, I try again and suggest let's at least leave a comment for the next poor soul running into that bug so they won't be banging their heads in the wall 2hrs straight trying to figure out why store X isn't there unless you call something last or never call it or shit like that (the sneaky slimy nature of that memory bug) .. He even dismissed that and rather went on saying (almost literally again): It is just that Mr. Senior had to rush things and communication can be problematic sometimes .. (bla bla bla) back in "Sunken Ship Co." days, we had a team from open source community .. then he makes a very weird statement:
Stuff like what Richard Stallman writes in Linux kernel code reviews can offend people ..

Feeling too grossed and having weird taste in my mouth I only get in a bad hangover day, all sorts of swear words and profanity running in my head like a wild hungry squirrel on hot asphalt chasing a leaky chestnut transport ... I tell him whatever floats your boat but I just feel really sorry for whoever might have to deal with this bug in the future ..

I just witnessed the team giving birth to a sneaky slimy bug .. heard it screaming and saw it kicking .. and I might live enough to see it a grown up having a feast with other bug buddies in this stinky swamp of Uruk-hai piss and Orcs feces.

PR comment: You don't need this useCallback and this useMemo, it's premature optimization, this component will not have a lot of rerenders anyway.

*remove it and merge it*

Few weeks later...
Slack message: why the app is so slow???

*opens a PR putting back all "premature" optimizations*

PR comment: Hum... I don't think that's it, but let's try it.

*merge it*

*slowness is gone*

Slack message: wow! How did you figured out so quickly???

Me: I was lucky, I guess ¯⁠\⁠_⁠(⁠ツ⁠)⁠_⁠/⁠¯

The current project I'm working with had 3 devs including myself until Jan 1st. Now we're only two, because our lead/manager started to work in other projects and trusted us.

Since that happened, my first PR/Commit of the year was in Jan 5, and it's still open, without any kind of review or comment, as well as my other five (eight in about a day) PRs, while he's making commits directly into develop/main branch, causing conflicts everywhere on what I did...

I'm leaving on friday because the contract is ending.

Good luck I guess.

Today I got a reply to a PR comment from a coworker literally stating that "it's not a good practice to add unit tests to hotfixes".

I can't, just can't. Left me speechless. Wonder where this guy gets his "good practices" from.

That feeling when the code in a massive PR is of very high quality... Only comment is about some class name...

PM tells me to merge (large feature) work that's heavily under WIP.

In that is a performance optimisation he instructed (with an aggressive DM) me to stop looking at even though it's a concern for larger clients that I had to fix before as it's "unnecessary" - thought whatever I'll leave the code as is then

I tell him him the PR is not ready yet, there is still a lot of clean up to and tests to write

Just do it

A ~week later "wow you write really selfish code like look at this"

Shows a wrapper class from the optimisation with 2 properties and getters and setters (and override some of super's properties). I explain to him why it's there, "that should have been a comment". I tell him I write detailed comments as part of my refinment process which he wanted me to stop.

This is after he tried to merge a release branch into main while sneaking in some "corrections" and I pointed out it breaks Dev.

So working for a company and the dev team I’m apart of works on a legacy rails app. Technical debt is high, no automated tests, no proper routing and also running unsupported versions of the language.

I joined seven months ago and got the current team doing automated testing so that’s a plus, they bought this app four years ago and there’s been no language updates, testing, cleanup, security updates, nothing, just adding to bad code.

Now we’re looking to actually upgrade language versions, the language and the framework now this will cause a lot of stuff to break naturally due to how outdated it is.

So I started putting proper routes into place how things should of been when things were being built as we have some spare time I decided to go out of my way to clear up some of the technical debt to get ahead of the curb. Re-done an entire section of the app, massive speed improvements, better views, controller, model, comment clean up and everything exactly how it should be.

I push the PR,
*other dev* - “why are we doing all of these other changes”
*me* - “well to implement routes properly, we have to use the new routes I just did some extra cleanup along the way”
*today, me* - “can you lend me a hand with one of the routes the ID isn’t getting passed”
*today, other dev* - “this wouldn’t of happened if you didn’t redo all these files, let’s just scrap the changes”
…

Sooo, I’ve spend three weeks improving one section in the app, because I’m having issues with one route according to this dev I should scrap it? Wait come again, am I the only one in this team who cares about making this app better all round?

Frustrating…

Why comment on the same thing during code review??

I submitted a PR and had to make a design choice that propagated throughout the module i was working on.

During code review, my coworker commented on every...single...line that this change effected asking "why are we doing x here?" instead of just creating ONE SINGLE THREAD with this question for discussion. There were at least 10 review comments on github from their one review that said "why X?"

Is this normal? Ive only had a few programming jobs and this is the first time this has happened to me.

personally, when someone makes a choice like that, i just make a comment and save the rest of the review until that is addressed.

To fix a bug I added a few log messages to trace what gets executed and in what order (very new to the project). Fixed the bug, pushed PR and the only comment was to remove the log files. 🤦‍♂️🤦‍♂️

Please tell me this is normal or should I start looking for a new place that hires "only the best" 😭

I don't get why this retard keeps ignoring my comments on a PR he asked me to review. He is a level above me so I can't push it. I just approve his PR and let him know I do so but with a comment. He then ignores the comments and merges his rubbish changes into the codebase thereby creating a tech debt.

is it necessary to have cherry picking a part of git branching/release process?

we have 3 branches : develop, release and master.

currently every dev works on feature as follows : they make a branch out of develop, write code, raise pr against develop, get it reviewed and merge back to develop. later the release feature list is generated, and we cherry pick all the release related commits to release branch, and make a prod build out of release branch. finally, the code is moved to master and rags are generated accordingly.

so the major issue with this process is feature blocking. as of now, i have identified 4 scenarios where a feature should not be released :

1. parallel team blocker : say i created a feature x for android that is supposed to go in release 1.2.1 . i got it merged to develop and it will be cherry picked to release on relase day. but on release day it is observed that feature x was not completed by the ios dev and therefore we cannot ship it for android alone.

2. backend blocker : same as above scenario, but instead of ios, this time its the backend which hasn't beem created for the feature x

3. qa blocker : when we create a feature and merge it to develop, we keep on giving builds from develop branch adter every few days. however it could be possible that qa are not able to test it all and on release day, will declare thaf these features cannot be tested and should not be moved to release

4. pm blocker: basically a pm will add all the tickets for sprint in the jira board. but which tickets should be released are decided at the very late days of sprint. so a lot of tasks get merged to develop which are not supposed to go.

so there's the problem. cherry picking is being a major part of release process and i am not liking it. we do squash and merges, so cherry picking is relatively easy, but it still feels a lot riskier.

for 1 and 2 , we sometimes do mute releases : put code in release but comment out all the activation code blocks . but if something is not qa tested or rejected by pm, we can't do a mute release.

what do you folks suggest?

Coworker: "Hey, so I discovered this library that automatically brings up and tears down local containers to perform unit tests on data sources"

Me: "Sounds neat"

Coworker: "Yeah, I've been messing with it locally, and it means we don't need to have the data sources installed on our machines or rely on the ones in the testing environment."

Me: "That's good"

Coworker: "Just a shame I had to roll back our testing framework to a previous version and refactor the code in all our other tests as a result."

Me: "Wait what? *looks at documentation* It says they support the newer framework"

Coworker: "Yeah, but I couldn't get it to work. So I'm just gonna make a PR for it, okay?" *Proceeds to make a PR, approve and merge the code before I can comment further on the changes*

Welp, there goes all my motivation to get anything done for the rest of the day.

Had a shameful moment today, when I got my PR kicked back with a comment "can you be patient and not rush this through?"

I knew that the person meant well and the code was not up to my usual standard but at the moment I felt a strong sense of inferiority. Worst thing is that it's still lingering as I am going to bed.

Hope tomorrow is a new day.

Man I'm annoyed!

TL;Dr what does it mean "we're trying to reduce options to a minimum", why don't you go closed source!? why don't you remove themes!?

For anyone who uses rofi, they would know that a few months ago an update made it more compliant with the free-desktop spec, that it only uses the first .desktop file for the given Name tag.

I only found out about this recently as I was only able to update Manjaro recently, and it really annoyed me, cause it took me a while to figure out why tons of my desktop entries disappeared.

Turns out someone made an issue about this, and the given answer was: "that's against the spec". Ok, fine. But when I asked if they could add an option to still ignore that aspect of the spec (i.e. --show-duplicated), the response I got was: "going against the spec is a no-go". WHAT!?

There are so many things that have behavior that goes against the spec (ex. gnu-utils), why can't they add an option to do this!? An OPTION!?

When I decided to try (I don't know C yet) and make a PR, the first and last (it got locked afterwards!) comment I got was:
" As explained on #941, this is a no-go. We want to reduce the number of options to the minimum, and non-compliance to a well-defined and widely implemented spec is definitely not something we want."

Why are you so closed minded!? Yes compliance is amazing, but it's not a safety standard, it's okay if you *give an option* to go against the spec!!!!

WHAT THE HECK!?!?!? WHY!?!?!?

Why is a open source project closed to new features that are part if the scope of the project, and require minimal maintenance!?

MRW the only comment a coworker puts on a PR is for indentation.... in Go..

Comment in a PR to use ur little component in the 50 pages of docs because it looks better. Fine.

Write 30min after i submitted the changes that it doesn't look as good as you thought and you can go fuck yourself fr. Should have tried it before asking me to change it everywhere mf

(after morning coffee) Today I’ll change the world and write the most elegant and revolutionary piece of code ever.
(end of day, after 8 zoom meetings and 5 gdocs comment rounds) guess i’ll do these 2 PR reviews tomorrow.

OpenSource is fun they said. I being a bored teen thought, ah, another chance to experiment. Discover something new. Now I am into piracy, movies, music, software. If I can get it for free I ain't paying for it. So I went on to GitHub to see what exciting new Repos I could contribute to. I hate already implemented plenty of algorithms in GO for GitHub.com/TheAlgorithms so I was looking something more practical, more beneficial to society. Then I saw it, the perfect repo, not too complex and not amateur. SpotDL/spotify-downloader for downloading songs from Spotify, a grey area coz it's technically piracy. Well not from Spotify, we fetch the info from the Spotify API and search for the songs on YouTubeMusic. They were just about to release v3, a complete rewrite of the codebase stressing code readability and stuff. I spend about a day studying the codebase, trying to findout just where I could make my contribution. I can see outright that there's a huge problem with implementation.

First of all the script spawns 4 processes for downloading songs though you might be downloading only one song. Which means for everytime you run the script you have to wait for 4 other processes to be spawned before any downloading can happen. Sure this is faster when you are downloading more than like 4 songs, but it's actually slower when downloading a single song. But I ignored that coz I assumed that most users download playlists and albums. Anyway we talked with the like lead developer and he was all like, make those PRs anytime you feel like. So I made a really minor first contribution.

I introduced download from Spotify URI functionality, modified like 10 lines of code. I was half expecting that the PR would be merged within hours at most 24 hours coz of how minor of a contribution it was, 5 days in it was pending. So I tagged the lead Dev and he was all appreciative of the PR, calling it real 'clean code' and stuff. 3 more days, the PR is still not merged. I have now stacked 4 more commits to the same PR, I tag the dev and he's like he's waiting to see if my 'feature' will get atleast 10 upvotes so that it can be merged, he links an issue. I go to the issue and my feature is not there, So 11 days after I made my PR I have to write a comment explaining the 'feature' introduced in my PR and then wait for 10 upvotes.

I was like f**k this, I'll just develop on my fork if you want the features on my fork, you will make your own PR! I am so done with OpenSource, development is slow. I have no idea how you guys do it. I can't handle development where I don't have write access.

Today I had a look at the Linux kernel on GitHub. There was a commit made by someone else which I tried to comment on. But my comment was not getting added— probably because it is not my PR. Is this a feature or a bug ?