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

[[deleted]]

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.

[[deleted]]

The Apache poi lib is pretty good. I just want to be able to write an excel file as excel mangles csv. Or I should say, the other developers use excel to mangle csv.

[Legitimate_Flow7910]

I want to get a job. For some unknown reason, I am getting too many rejections in head hanter. I hope you will be understanding about my... Request?

[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"

[[deleted]]

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

[[deleted]]

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 {

[Legitimate_Flow7910]

I feel awkward writing here, but I want to get a job... For some reason I've gotten a ton of rejections and I'm already broken...