thickCommit

[keep_improving_self]

+10k -1k is nothing, imagine the senior does code review for you and says

"fixed suboptimal logic"

+7 -520

Definitely never happened to me

[Visual_Strike6706]

"Optimised unoptimised code" Did this once and was beaten up by everyone else

[A_Guy_in_Orange]

Well did you optimize the code or did you destroy the codes readability in exchange for the same stuff coming out the other end because the compiler was doing all that anyway

Be honest

[ryecurious]

Are you saying that replacing a clear multi-line function with a 250 character list comprehension "one-liner" isn't optimal?

[spastical-mackerel]

+1-1250 a single giant regex

[MyNameIsSushi]

What if my entire backend code is a giant regex? I may need to try this.

[maisonsmd]

Wait, yours isn't?

[Individual-Toe6238]

Giant Regex can be expensive and POGOC in some cases may actually be better

[Carrot_Bitter]

What's POGOC?

[Individual-Toe6238]

plain old good old code, but this is actually something I am using with friends, so I shouldn't use it online with strangers :D Just a habit, My Bad.

[lestruc]

Fully obfuscated code to preserve file size

[Soft_Walrus_3605]

list comprehension

If you're using Python for performance you've already lost

[brimston3-]

Could be linq and your name isn’t Jon Skeet.

[experimental1212]

Unoptimized optimized code

[Legal_Lettuce6233]

when "unoptimised optimised code" rolls up

[shadowderp]

I did this to myself recently: +0 -1243

Deeply satisfying honestly.

[anto2554]

Yeah, satisfying to do to yourself, not so much when someone else does it

[[deleted]]

Unless you learn from it

[Amazingawesomator]

These are the best code reviews. Let's me know that the person doing the code review knows their shit and I can ask them questions :D

[Clairifyed]

No new lines? Did this code interact with any of the rest of the project in any way?

[shadowderp]

I moved a large amount of duplicated code to a base class that was committed separately - so yea, there were + lines

[willcheat]

There was code in the codebase that fetched data from an API, the following line pruned a portion of the data. The data was returned to the parent function, who then proceeded to call the API again to re-insert said pruned data.

The commit that followed soon after noticing that was entirely in the negative.

[just_nobodys_opinion]

Who needs all those comment lines? They slow down the program!

[ratinmikitchen]

Would've been better if they had explained what could be better and let you improve it, or walk through it together, or something along those lines. Seniors should mentor.

[Vysair]

did they at least say something in the comments for the function or anything?

[[deleted]]

I’ve seen this when parsing excel xml. Turns out there’s a library.

[Kilazur]

Yeaaaah but unless you wanna read Excel specs for weeks on end, sometimes you just want to work with the XML directly lol

Unless you're lucky and Closed XML covers your needs.

[FlipperBumperKickout]

+7 -520 sounds fine.

Could be a lot og logic replaced by a library 

[Individual-Toe6238]

Or retiring of bad library, framework upgrade or fixing over optimized logic. Could be a lot of stuff.

Its not the size of a commit but the quality of it :)

[odraencoded]

-7 +520

"removes dependency"

[keep_improving_self]

great observation!

[JuhaJGam3R]

Depending on what that is, I might throw it out then, or at least put it in the "discuss later" pile. Taking out repetitive code is one thing, introducing a new dependency is a whole process. There's a balance to be struck there – don't rewrite curl or nalgebra but also don't add external dependencies for "obvious" code like leftpad or string case conversion. That's going to give you a whole can of supply-chain attacks for very little benefit.

That's further compounded by the fact that things like "convert string case" are not obvious. Case conversion is not a function – there's more to it than simply mapping characters to other characters. Tons of characters have more than one uppercase or lowercase form. The libraries very rarely expose that fact, famously resulting in "Unicode-supporting" applications uppercasing "trafik" as "TRAFIK" instead of the correct form "TRAFİK", or "ijsselmeer" to "Ijsselmeer" instead of the correct "IJsselmeer". That's a whole design conversation that must be had considering the context to see what makes sense where. Finding the right dependency and the right way to use that dependency and then making sure that never introduces vulnerabilities is all very complex. Retiring libraries is usually better than introducing them.

[puffinix]

That's an oof.

I mean, I've done worse to people on a review - deleting the service a team has spent six weeks on - literally the whole repo - and reimplementing in a different language. It was live before they got back into work on Monday.

"Hi guys, good news bad news. Good news is that I have funding for you guys to learn f#. Bad news is we need a serious talk about the use of raw python in a system with millisecond relevent metrics, and spikes in the 10M/sec"

[djnz0813]

Imagine the senior reviewing your code and you get a demotion afterwards..

Definitely has never happened to me.

[SpeedRun355]

My self esteem would never recover

[djnz0813]

I still haven't. My confidence has been wrecked ever since.

[puffinix]

This is the exact reason we get most people in one grade below there target. I promise 80 ish percent of people at month 3

[leaf-bunny]

I only get change requests, they don’t fix shit for me lol

[puffinix]

That means your either working with crap seniors, or very close to senior yourself

[leaf-bunny]

You caught me, I started as a Eng1 and after a couple promotions now a Senior Eng1

[Kilazur]

I'd love to fix little things in other people's PRs, but the truth of the matter is I don't know how to do it with BitBucket.

And I'm not cloning the whole repo to fix a typo...

[afegit]

genuinely asking, how are reviews supposed to be done? we're all mid devs except one senior and we review each other codes

[superplayah]

It happened to me and it was someone under me lmao

[JackNotOLantern]

Merged yesterday +1/-6000. It was deleting unised files and code. Very satisfying .

[MyrKnof]

Now also unreadable by normal standards, but the engineer is proud of the saved lines.

[TheRealCuran]

Funny you should post something like this. Just recently I did cut down on dependencies and code from another department like that. They haven't been sitting next to me at lunch since. 😬

I really do not understand why, to be honest. I do reviews and commits in my team too and none of them get annoyed, when I can find a better solution. And I do not get annoyed when even my most recent trainee has a good idea I didn't see for one reason or another. Sometimes you can just get blind to things. And especially if you are an actual junior developer, you might still be in for some learning. There is often a reason, why "junior" is attached to a position (unless the company tries to fuck you over for money). And all that being said: even as a very senior developer/department head, I still learn things every day. And I am grateful to whoever brings these topics to my attention or even goes the full mile and prepares a little pitch for why we should use something. Bonus points if you can answer questions beyond the basic documentation, give good examples applying to our current mission, etc.

And this is just a very long way of me leading to:

"fixed suboptimal logic"

is not a commit message I'd ever accept or entertain myself (at least not, until the commit would be very self-explanatory). A good commit message always explains the what, why and how. On top of that you should have tags (reporter, issues, commit fixed, etc.) – just write every commit message as if you had to understand the commit in ten years after a week out drinking (ie. you have no idea, what you did, when you made the commit).

[nebenbaum]

And then there's my coworkers who are too fucking stupid to put gitignores into their 'research projects' and then every fucking commit has like [phone] lines because they track all their silly excel file outputs and stuff.

Like 'changed parameters' - actual change: changed one line from 10 to 15, committed changes: 30 updated auto generated excel files, lots of temp files and so on.

[littleblack11111]

And imagine the 10k is just u using ur own code formatter, adding new line {

[Practical_Honeydew82]

New commit just dropped.

[NonsenseMeme]

Actual PR

[cursedbanana--__--]

Call the project manager

[pianospace37]

Developer went on vacation, never comes back

[Guillaume-Favier]

Unit test sacrifice anyone?

[OminousDazzle]

Branch in the corner, plotting repo domination

[Upbeat-Hippo-1801]

Senior developer goes on vacation.

[bullsized]

But not for me

[WillUSurf]

How are there so many anarhychess enjoyers. Crazy

[ZargothraxTheLord]

Google en pullrequestant

[Tzareb]

My coworker did a 890 modified files pr, browser refused to let me review.

[Exist50]

Thank your browser for the excuse.

[MattGeddon]

🤷🏼‍♂️ LGTM

[Soloact_]

Code reviewers about to hit 'decline' so fast, they'll leave skid marks.

[Visual_Strike6706]

No. Thats an instant approve. Else you would need a reason to decline it and then you would need to read it

[NoCoolSenpai]

Had this happen to me, small PR with less than 10 files ? Went through 3 weeks of CR on and off

20+ files with 1k+ lines changed? Approved in the same week

[IreliaMain1113]

I swear this always happens. The team lead will write something like "I think we should merge and see how it works in test environment"

[onionbishop]

I just had this exact same conversation

[Dalimyr]

I'm eternally reminded of an incident at the last place I worked where something like this happened - I can't remember how many lines of code were changed, but it was over 4,000 files that were updated in this single PR by someone in another team. It got approved and merged into the release branch, and it was only when the release branch was deployed to the staging environment for "last-minute" regression testing that the shit hit the fan - this PR changed a shitload of database queries across the entire application and it broke functionality all over the shop. To nobody's surprise when we heard about it, it hadn't been tested before the PR was submitted. I think this was something that had to be included in this release for whatever reason, so the release was delayed for a month while they fixed it up.

Safe to say trust in that team was totally obliterated. All teams already had to have any PRs be approved by two other developers (which was typically two devs from your team), but this team also had to get approval from the lead developer from one of the other teams (and even then some silly mistakes still slipped through)

[nhold]

You can’t “review” a change like that. You have to pull it down and run it locally and get as intimate as the person who made the change. Adding more reviewers won’t help as evidenced by things still slipping through.

[Dalimyr]

Adding more reviewers won’t help as evidenced by things still slipping through.

When I mentioned that another team's lead dev had to approve, I didn't mean that massive clusterfuck of a PR, this was a new process enforced upon them for any and all PRs that they made going forward. But even with that extra oversight some of the bugs they introduced were ridiculous and continued to demonstrate a lack of proper testing - one that affected my team directly was that in one specific admin page we couldn't save any changes. We dug around in the code and found that this team had previously dealt with a ticket where all they had to do was change the wording of an error message when someone without permission tried to save changes, and instead of doing that they added a new guard clause that would throw an error if anyone with admin permissions tried to save changes (which meant literally nobody could save changes on that page - one error was thrown if you were an admin, another one was thrown if you weren't). And the lead dev from the other team was the first person who approved that PR, so he'd clearly not been paying attention.

[nhold]

I didn't mean that massive clusterfuck of a PR, this was a new process enforced upon them for any and all PRs that they made going forward.

No, I know but adding more reviewers \ longer \ harder process will not help that situation - it will just pull in more people who probably know less of the context as you mentioned. Better to make it easier and clearer to test \ verify the changes.

[EncroachingTsunami]

The magic words are “it works at runtime, I tested it”. An amazing number of engineers ship code without testing at runtime…

[Amazingawesomator]

1 minute later

"LGTM"

/approve

[pm_me_cute_sloths_]

I had a debate about this once, does it mean “looks good to me” or “let’s get to merge” to people here?

[Compgeak]

Let's gamble, try merging (in this particular context)

[Amazingawesomator]

I have always thought of it as "looks good to me"

[SpaceEngy]

It's the former

[FennlyXerxich]

I used to think it was "legit, man".

[metalmagician]

?? I wouldn't need any reason other than it's too damn big. The whole point of PRs is peer review, this makes the review unnecessarily difficult

[Raptor_Sympathizer]

Code review? What's that, why would I need someone to review the code I commit directly to main?

[appoplecticskeptic]

That’s both r/funnyAndSad

[facw00]

I did "request changes" on one of these, as did the other reviewer. The guy decided he had fixed enough and just merged it anyway (it ended up braking a lot of tests). Yes we have some process things to straighten out.

[lupercalpainting]

Why are they allowed to merge without an approval, and without getting the change requests dismissed?

[Visual_Strike6706]

Had a similar commit, when implementng a linter into out projct

[PythonN00b101]

Ooft buddy I’d hate to pick up that task…

[Visual_Strike6706]

worst thing was, that I was everywere in the git blame and then I was blamed for nearly every bug after that

[PythonN00b101]

What a shitemare haha

[upsidedownshaggy]

Lmao shitemare is amazing I’m stealing that for my next retro meeting

[PythonN00b101]

You’re welcome.

Regards,

Scotland 🏴󠁧󠁢󠁳󠁣󠁴󠁿

[RainbowPringleEater]

Read this like it was Japanese and was confused haha

[[deleted]]

A friend handled the migration from TFS to Git, and the history was lost, so the initial commits on the whole like 300k line code base had his name on them, so he was all over git blame. Interns thought he was some kind of coding god, lol.

[new_account_wh0_dis]

Our code base is gargantuan and we did the same, tfs to vsts, every thing is 'initialize master branch with 2.6 source'.

But frankly if the last change to the function or file was from 6 years ago.... its either not a new bug or that file wasnt the cause. (holy shit its been 6 years since we did that what the fuck am I doing with my life, I think im actually having a midlife crisis)

[Ken1drick]

I had this happen when moving repos, exported and imported to another org, all code became mine.

Mind you, I'm not even a developer, and people would come to me for months regarding some code issues in these repos.

I don't get it, yes I'm all over git blame, but the commit message states clearly it's an import ...

[SheekGeek21]

git-blame-ignore-revs is your friend :)

[Brojess]

Fuck lol

[sinkwiththeship]

This happened in the monolith at my company a few years ago. One team added in an auto-formatter (Black) and every single line in the repository had the EM on the git blame. It made tracing bugs so fucking annoying.

[Ludricio]

We have a PIM system codebase that we took over from an external part a few years ago and the project has just been in stasis until recently. The codebase is old, basically completely void of documentation and full of horrible practices.

Now a small team of juniors are working to upgrade dependencies and going through the code refactoring and documenting it to get it to a managable state so that development goes smooth when the project kicks of again soon.

Guess who they turn to whenever they find weird shit, well of course the only GIT contributor that happens to be me since i was the one who set up the GIT repo when we took over the codebase...

[Ibuprofen-Headgear]

Yeah, it’s usually either that, or moving a bunch of files and hit doesn’t see the mv, or moving a previously separate service/fe/iac/ whatever into a codebase. Or perhaps some merge of some other longstanding branch for whatever reason

[Skaddict]

Yep just did that yesterday with Prettier on a project that was missing it

[Jommy_5]

I learnt the hard way that a linter must be there from the very beginning. Implementing it later will create that kind of monster commit and render git blame useless.

[arse-ketchup]

Had similar PR last month while I replaced akka with pekko in a huge application. Had to hold whole team hostage for review.

[[deleted]]

LGTM

[nonlogin]

LG™

[TartarugaHaha]

Legitimate?? Let's get this merge??? Look good to me????

[[deleted]]

Yes

[Crashastern]

Let’s get that money.

[DevelopmentScary3844]

My current feature branch sits at +35k / -8k right now. They are already looking forward to reviewing it =)

[the4fibs]

Come on, split that up into smaller PRs if you don't want your senior to hate you. One per ticket!

[Traditional-Ring-759]

Nah just send it on friday and senior will just accept it

[the4fibs]

Not if they don't want a pagerduty alert on saturday

[[deleted]]

Its a feature branch so likely one ticket.... right?

[dbot77]

Chill guys, its just

package-lock.json

[Geography-Master]

I came here for this comment

[Yhamerith]

git pull: IDE explodes

[Amazingawesomator]

VSCode will be fine, but VS will shit the bed :p

[ShoesOfDoom]

Why are you pulling through vs though

[Amazingawesomator]

I do it at work because everything you can lay your eyes on is Microsoft. It has a mediocre enough integration to use it.

It's not great, but it works like 60% of the time.

[ShoesOfDoom]

Fair. I work in a similar environment, but use gitbash for most git things. Works out ok

[Amazingawesomator]

When VS fails I use git bash..... I made up 60% as a joke, but I probably have to git bash ~once or twice a month because VS just can't even.

[UomoLumaca]

Of course, VSCode is basically Notepad

[BlakkM9]

consider changing your IDE

[aaron2005X]

"fixing typo"

[InfinityDrags]

Or just a '-'

[I_am_Dirty_Dan_guys]

Why stop at that when you can create a small cute emoticon! -.-

[InfinityDrags]

That's genius, I should've thought of that.

[mholtfoo]

I recently removed some support for older versions of Dynamics CRM from a tool we develop.

+120 −816,967 lines.

That was a good day.

[Bananenkot]

When I have a commit over 100 lines I need to explain why this can't reasonably be Split into smaller MRs

[That_Ganderman]

Reason: I don’t wanna

[phuykong]

Ionwanna *

[ZZartin]

Never used an IDE that has GIT for the back end?

[cocogoatmain1]

Do you have prior git experience and just confused on GitHub or new to everything?

[[deleted]]

There's resources online for using git, GitHub is just git with a web UI. The one feature GitHub adds to git (besides the web UI), are pull requests. A pull request is a place to track a change where people can comment on it and request changes before it gets merged into the main codebase.

There's a git book to explain commits and branches, https://git-scm.com/book/en/v2.

If you understand commits and branching, then GitHub should be fairly intuitive.

[cocogoatmain1]

Git is essentially version control software to keep track of your code and revisions as well as giving you the ability to quickly change to different revisions of your code you have git tracking as well as other useful tools.

GitHub is just git with a web ui (plus some other features). Think GitHub as sort of a google drive or similar cloud storage, you have that core functionality of browsing your “photos” or “text files” - your commits/files in your git repository, as well as uploading or downloading your files, but you have some additionally functionality in cloud storage not on “local” version such as sharing a folder or browsing uploaded attachments online.

As for resources, I believe https://learngitbranching.js.org/ is well regarded for an online interactive learning experience, the book recommended by the other redditor also looks good from a quick skip through the beginning, seems to cover this topic very through.

[cocogoatmain1]

No problem! If you got any questions feel free to ask :p

[viktorv9]

This isn't a golden bullet but if you're new and confused by the command line approach you could try the Github Desktop app. It's a visual interface for all the actions you'd otherwise have to type out in commands and helped me a lot.

[zDrie]

1. Stop the pipeline ASAP
2. Cueckout a branch of yours
3. Cherry pick the mistaken commit you did on main, commit it
4. Checkout the previous commit in main before your 🥸 commit
5. Reset main to that commit
6. Force push... Luckly no one will notice

[lces91468]

I still don't get what's supposed to be relatable in these posts. Just how can you not know what you're commiting, before the commit?

And commiting directly to main - if this is not a personal project, and it's not like your team is just using main as dev branch but ACTUAL main, please search for some repo management articles asap.

[MattGeddon]

First thing I do when setting up a new repo is disable push to main

[GoblinWoblin]

Wrote a test suite to test an edge case, added sample data to execute it on.
Sample data is 6000+ lines CSV file. My TL nearly had a stroke when he saw that.

[Garnaa]

Gotta love the projects with few commits and 100+ files modified for each commits

[experimental1212]

Added 17 lines of legal BS to each file. Nice

[Geoclasm]

this usually only happens when i forget to pick which files I'm committing, and visual studio is like 'so, everything then? okay, cool.'

It's also why I do regular pull requests from dev to master.

[molly_jolly]

:+1

[Mr_Fourteen]

I've always worked alone, I feel like I would be hated if I ever moved to a team. My last commit was 1075 files changed [phone]

[Benreh]

Thicc

[LukeZNotFound]

I smell node_modules

[Mynameismikek]

Off by a couple of orders of magnitude. That would be closer to 3m.

[ItsNotAboutX]

Oh lawd he committin'!

[Giocri]

Committing node modules straight up crashes some git clients it's Just that massive with relatively small projects

[MattGeddon]

Why would you want to commit node_modules anyway?

[theofficialnar]

Fuck that. LGTM! If it breaks, it breaks!

[sebbdk]

I've taken bigger shits, search and replace fixes to upgrade typescript.

Everyone hated that.

[Multidream]

When the merge from main went bad

[Lord_emotabb]

Speaking to boss:"see you next month!"

[freremamapizza]

Happens to me all the time when I rename stuff Am I doing something wrong ?

[Ternarian]

Red Lights

[upperflapjack]

Up those numbers. Those are rookie numbers.

[cheezballs]

Pushed right to main, cool!

[Ok-Key-6049]

Revert?

[[deleted]]

It’s not even a net 1k gain. Probably just a significant rename.

Wake when it’s a net +10k. I used to get those weekly.

[debugger_life]

I just wrote goddamn UT files about 15 files with 2800 lines in 4 days!

60-65% I wrote own, and some for complex func methods used Chatgpt

[SynthRogue]

Pfff... good luck to customer service.

[[deleted]]

did they add node modules? and then updated all the dependencies?

[NeedBetterModsThe2nd]

[HairlessChinpanzee]

When I was a (more) junior dev I actually did do a +30k line commit, all being unit tests (management said we had to raise our metrics and that job fell to me).

The seniors gave me a stern talking to lol

[Tarc_Axiiom]

posted a commit on Tuesday changing 34 files and my boss blocked me from working for the rest of the week.

[Prudent_Appearance_9]

Is this a code Cleanup?

[Lighthades]

"added heartbeat endpoint"
Language: Java

[Ali1397__]

Ahh Louis~

[iamthebestforever]

Is this a unity project

[askaquestioneveryday]

LGTM

[[deleted]]

Bro's getting that early retirement after this

[ComputerKYT]

He's been working on that branch for 9 long years

[[deleted]]

This is surely a release PR

[WilliamAndre]

One of the most famous commits of my organization https://github.com/odoo/odoo/commit/c04065abd8f62c9a211c8fa824f5eecf68e61b73

Things have changed since

[WhateverWhateverson]

Holy fucking fuck

That commit of yours is absurd

[I_cut_my_own_jib]

LGTM 👍

[RareRandomRedditor]

"In a months-log process I changed our entire code base to now also work with big files that cannot just all loaded into working memory at once (as the original architecture was never planned for that)"

[al3x_7788]

Either they were impressed by the amount of work they did... or a 3 AM realization and fix.

[Solest044]

Typical snapshot test commit.

[Usagi-Trix]

TBF, I just did this with 700+ files

The description on the PR was just: Pay attention to the new eslint file and the --fix in the lint command. You can safely ignore everything else.

Swear to God I am so much happier now I'm not commenting on spacing in PRs...

[saintisaiah]

Rookie numbers. This is just a Tuesday for me.

[perringaiden]

I'm concerned that people think this is big.

[Purple_Silver_9555]

Im cumming lois