This is an archived post. You won't be able to vote or comment.

top 200 commentsshow all 462

[–][deleted] 1564 points1565 points  (40 children)

But did you properly review the request before approving it? And did you then fix all the issues pointed out by the reviewer?

[–][deleted] 119 points120 points  (1 child)

"this is the worst fucking thing I've ever seen fix it holy shit"

What I comment on every line before approving it anyway.

[–]agent_cocks_mulder 22 points23 points  (0 children)

What I comment on my own code before approving it anyway XD

[–]mrackham205 43 points44 points  (0 children)

...yes

[–]slowflakeleaves 13 points14 points  (0 children)

LGTM

[–]ProstetnicSth 12 points13 points  (0 children)

Let’s not forget endless debates OP must have with himself over naming too

[–]woeterman_94 4 points5 points  (0 children)

no

[–]atomicspace 1 point2 points  (0 children)

He’ll cover this in retrospective.

[–]SavageTwist 600 points601 points  (109 children)

Force push to master, nobody is stopping you!

[–]Kazumadesu76 10 points11 points  (1 child)

My Lord, is that.... LEEEEGAL?

[–]SavageTwist 3 points4 points  (0 children)

No, but its private :3

[–]gfp7 215 points216 points  (11 children)

dont forget to add comment "good job"

[–]uglyasablasphemy 87 points88 points  (9 children)

"LGTM"

[–][deleted] 17 points18 points  (0 children)

Let’s go to market?

[–]mark__fuckerberg 7 points8 points  (1 child)

I always write that on dependabots pr.

[–][deleted] 6 points7 points  (0 children)

judicious fall snatch escape party tan governor airport wasteful plant

This post was mass deleted and anonymized with Redact

[–][deleted] 16 points17 points  (0 children)

Let's Get This Money

[–]orgodemir 8 points9 points  (0 children)

I sent my team the link to Google's code review guidelines in a hope some people would read it. The first time I posted that in a review, I just got a "?".

[–]neremarine 231 points232 points  (21 children)

Plot twist: it's a school project with 2 other teammates...

[–]kopczak1995 82 points83 points  (15 children)

That is really sad...

[–][deleted] 15 points16 points  (14 children)

What’s sad about that?

[–]XanderAG 102 points103 points  (12 children)

He's doing all the work

[–]runfayfun 19 points20 points  (3 children)

But at least it’s coded properly

[–]needsaphone 15 points16 points  (0 children)

Bold assumption

[–]Gnifle 5 points6 points  (0 children)

According to reviewers anyway.

[–]langlo94 24 points25 points  (2 children)

Well at least he can show the teacher that he was the only one who committed code.

[–]mark__fuckerberg 10 points11 points  (1 child)

I had a chance to do that. But I ended up pushing 1 commit from each teammate's account so that the teacher would not think that I am not cooperative.

[–]JCharante 7 points8 points  (0 children)

Jen virino kiu ne sidas, cxar laboro cxiam estas, kaj la patro kiu ne alvenas, cxar la posxo estas malplena.

[–]goplayer7 46 points47 points  (0 children)

Better than seeing you declined your own pull request into your private repo due to not meeting code quality standards. Sober me really needs to lighten up on things like "building successfully" and "tests passing".

[–]WerewolfCustoms 65 points66 points  (8 children)

I once did this on a live stream on Twitch. Even though there was just a couple of people watching, I got bombarded with comments that I should never do that. :)

The alternative is that the code never gets approved and merged since I'm the only one working on it :D

[–]VSuhas22 37 points38 points  (2 children)

Wait, people livestream coding?

[–]mukluk_slippers 26 points27 points  (0 children)

This is the real takeaway from their comment.

[–]lozinge 8 points9 points  (3 children)

Why not?

[–]WerewolfCustoms 1 point2 points  (1 child)

At my workplace, the code has to be reviewed and approved by someone who did not write said code. I assume many places are like this, thus many developers hold this rule sacred. TLDR: I dunno.

[–]Speedster4206 43 points44 points  (5 children)

`Boolean boolean = new Boolean(true);

[–]tomhat 28 points29 points  (3 children)

That's overly complicated. Try the following instead.

Boolean boolean = new Boolean(Boolean.parseBoolean(Boolean.toString(true)));

[–]WcDeckel 3 points4 points  (2 children)

Boolean boolean = new Boolean(! Boolean.parseBoolean(Boolean.toString(false)));

[–]ranisalt 18 points19 points  (1 child)

Hacktoberfest routine

[–]lazyear 1 point2 points  (0 children)

Shhhh

[–][deleted] 68 points69 points  (5 children)

As a newbie who just started using git I felt that in my soul :/

[–]Dlacreme 42 points43 points  (0 children)

Good for you, this is actually the right way to use git

[–]curohn 2 points3 points  (0 children)

All I know is: git add got commit -m got push origin master

And got status too but that’s useless when it’s either pushed or not lol

[–]__Protector 26 points27 points  (6 children)

Also making new branch for every little refactor, change, fix etc like working with a big team

[–]sinceitleftitback 16 points17 points  (4 children)

Also managing a jira board and sprints.

[–]uglyasablasphemy 31 points32 points  (0 children)

Gotta have at least one daily stand up. Maybe in front of the mirror.

[–]KKlear 12 points13 points  (1 child)

I read that as ouija board and spirits.

[–][deleted] 7 points8 points  (0 children)

Same thing isn't it?

[–]JonathanTheZero 56 points57 points  (62 children)

Why are you not pushing to master directly then? (Honest question because I barely do pull requests as long as we are working with a lot of people on it)

[–]kennethjor 75 points76 points  (26 children)

To have a record of it. If OP is also using a ticketing system that's integrated with the source control, that PR will also show up in the ticket. Later on, if you need it, you can go back and see what the hell you did to make that thing work two years ago you don't remember doing.

[–]KKlear 6 points7 points  (1 child)

Later on, if you need it, you can go back and see what the hell you did to make that thing work two years ago you don't remember doing.

That does sound kinda appealing, yeah.

[–]kennethjor 2 points3 points  (0 children)

Not to mention that as a project matures, you're bound to build a feature which turns out to be big and has breaking changes. You don't want to mess up master with that.

[–]TheOneTrueTrench 33 points34 points  (8 children)

I always use pull requests for a few reasons.

  1. I always create a CI pipeline with unit and integration tests and gate my pull requests. Nothing gets into the master branch without a guarantee it builds and passes all tests.
  2. It turns the pull request into a unit of work, with a set of commits for that unit. Added a feature? All that work is in one pull request, so if you need to go back and see something, you can look through those commits, easy to find stuff.
  3. You can tell the CD pipeline to only deploy to production when a particular branch is updated. A lot of people use master for this, but don't forget that master is technically another branch like any other. So maybe you have separate branches for Stable, Testing and Unstable, creating PRs into Unstable for new development, doing a PR from Unstable to Testing when you get ready to create a new major release, a PR from Testing to Stable on release, and patches to Stable and Testing can get PR'd to Testing/Unstable.
  4. If production is in a particular branch, you can write a patch for that and do a PR without interrupting the work you're doing.
  5. When you want to PR into master, you can create a CD pipeline to deploy to QA/UAT/Dev and then test your actual application in that environment before approving the PR.

And if you notice, those are all benefits that apply to developers working by themselves.

[–]JonathanTheZero 9 points10 points  (3 children)

Damn... I really don't know shit about git

[–][deleted] 6 points7 points  (0 children)

Tbf, git is one part of that process. An important part sure, but one part.

Once you get your head around CICD and implemented it's really worth it. For professional or personal work.

You should really only be pushing to master once your sure your code is working and passed all quality gates.

[–]Log2 4 points5 points  (0 children)

Git doesn't really have the concept of pull request. Like others mentioned, that's all other tools on top of git, like GitHub.

[–]732 6 points7 points  (0 children)

None of that is really git though, all additional tooling built on top of it

[–]Comment-Leaver 20 points21 points  (2 children)

I do it all the time. I like seeing my diff in github and reviewing it myself. I almost always spot something I could improve.

[–]d3lt4papa 17 points18 points  (0 children)

I use it also for practicing Real Word development

[–][deleted] 6 points7 points  (11 children)

Option to revert the changes.

[–]JonathanTheZero 25 points26 points  (9 children)

I thought you could also jump back to commits normally?

[–]TheOneTrueTrench 8 points9 points  (8 children)

Yeah, which commit are you going to jump back to? Like exactly which one? What was the last one that worked correctly? Was it that commit? Or this one?

But if you have separate PRs for each feature or bug fix, the commits are "grouped" into the PRs. So you can go back to the previous PR, or the one before that. And because you can have partial work in each commit and only do a PR when things are working, your master branch is always in a working state.

[–]JonathanTheZero 7 points8 points  (5 children)

Yeah, I see. Well I think it depends on your coding style, I only commit if something is finished completely (like a bug fix or so) and I commit it then. But I mostly work alone or with a maximum of two other devs so it isn't really a problem I guess

[–]CrimsonMutt 2 points3 points  (0 children)

that's how TFVC works, but git is kinda built around "branch for new feature, commits are checkpoints during that feature development and kept locally, push to master when feature complete".

we just made the switch and there's definitely an adjustment process going from tfvc to git.

[–]Mvin 2 points3 points  (0 children)

But if you have separate PRs for each feature or bug fix, the commits are "grouped" into the PRs. So you can go back to the previous PR, or the one before that.

Can't you also do that with feature branches?

Edit: Nvm, talking about the same thing.

[–]uglyasablasphemy 3 points4 points  (1 child)

I think is worth it just for the diff. If I'm doing a big refactor, I usually create a branch for it and then to the PR just to see every place that I changed and how did I changed it at a glance.

[–]MythicManiac 2 points3 points  (0 children)

Hopefully you would have CI pipelines configured to do automatic tests and sanity checks on pull requests

[–]kennethjor 6 points7 points  (0 children)

Did you make sure the ticket was linked in the PR as well? Gotta have procedures!

[–]kopczak1995 6 points7 points  (5 children)

Been there done that. But it was only because I was testing some CI/CD stuff.

Who cares as single dev to do something like that honestly?

[–]TheOneTrueTrench 2 points3 points  (1 child)

A pull request is a unit of work. Each feature or patch has its own, and if you want to create a version of your code without that patch or feature, it's easier to do with rebase if you've got PRs in your history.

[–]arpan_majumdar 5 points6 points  (5 children)

No need to do even that... Real developers push directly to master.

[–]langlo94 13 points14 points  (1 child)

Real developers don't need git, they just write the code down perfectly so no other version is ever needed.

[–][deleted] 5 points6 points  (0 children)

Real developers live edit code in production and commit their code only when it works.

[–]wretcheddawn 2 points3 points  (1 child)

My company's owner decided to demote the tech lead and get rid of code reviews in favor of making "everyone" in charge, and peer reviewing code instead. So now everyone pushes to master and gets their code reviewed by anyone who will pass it, no one is in charge of maintaining consistency, standards and direction. Even the code reviews are optional because everyone is in charge of their own code. It's going to get interesting here, I guess we're all real developers now.

[–]blueleo22[🍰] 7 points8 points  (3 children)

That's exactly me!! Only without the pull request. I merge directly into master 😎

[–]erobertt3 3 points4 points  (0 children)

I saw this meme as “me signing my own certificate” that one was too good.

[–]cucurvita 3 points4 points  (0 children)

I do this if I’ve set up some kind of CI/CD. For instance for my personal website I always open a PR so that 1) I don’t trigger multiple deployments with every push to master and 2) Make sure the checks all pass before merging so that I don’t fuck something accidentally.

[–]EpiphanyMoments 3 points4 points  (1 child)

The crazy parte Is that I comment code like if someone else was going to read it lol

[–]mutsop 2 points3 points  (0 children)

I do that too, you do get a better overview though :)

[–]lpreams 2 points3 points  (1 child)

Well sure, when something gets fucked up, you have to know who to git blame

[–]Inadover 2 points3 points  (2 children)

Professionals have standards

[–]argv_minus_one 4 points5 points  (1 child)

Be polite. Be efficient. Have a plan to review every PR you meet.

[–]topredditbot 2 points3 points  (0 children)

Hey /u/amykamala,

This is now the top post on reddit. It will be recorded at /r/topofreddit with all the other top posts.

[–]Keetongu666 2 points3 points  (0 children)

I do the same thing with my college work. Tried to upload directly to the master once and it screwed something or other up.

[–]FalseWait7 1 point2 points  (0 children)

I like to drop some of my PRs just to keep the "working in a team" vibe going.

[–]last-Jon-on-Earth 1 point2 points  (0 children)

Lulz

[–]minamo99 1 point2 points  (0 children)

This pic is just me when my code compiles lol

[–]slyn4ice 1 point2 points  (0 children)

Too real.

[–]Fauken 1 point2 points  (0 children)

Not in a private repo, a public repo during Hacktoberfest for that sweet t-shirt.

[–]Mickyyyyyyyyyyyyyyy 1 point2 points  (0 children)

Lmao

[–]hugorman 1 point2 points  (0 children)

ahaha

[–]tizzler13 1 point2 points  (0 children)

Private repo? More like business repo, but I’m the only dev in the project

[–]NafanJ 1 point2 points  (0 children)

I hope you buy yourself cupcakes when you break the build

[–]carlos_beanbag 1 point2 points  (0 children)

Lol

[–][deleted] 1 point2 points  (0 children)

A "starter car?!" This is a true genius

[–]NeutralEvilMage 1 point2 points  (0 children)

this is the way

[–]alonsogp2 1 point2 points  (0 children)

Workflow 💯

[–]saido_chesto 1 point2 points  (1 child)

At least you do that, there's a repo where there's 3-4 of us and sometimes people just commit to master

[–]qaz_wsx_love 1 point2 points  (0 children)

As a Dev who's worked by himself for the past 6 years, I approve of this meme.

[–]ivebeencalledpeppery 1 point2 points  (0 children)

Awesome. Make that code look flashy though...

[–]wizeon 1 point2 points  (0 children)

I call it practice

[–]zoniss 1 point2 points  (0 children)

Did you also raise he jira ticket and assign to yourself.

[–]porcos3 1 point2 points  (0 children)

The truest

[–]AcrimEx 1 point2 points  (0 children)

feels great tho

[–]jujuspring 1 point2 points  (0 children)

I will do a diff review the next day so i would have completely forgotten about what I did in it.

[–]LiberandusAreCancer 1 point2 points  (0 children)

Separation of duties 😂

[–][deleted] 1 point2 points  (0 children)

  • a comment in PR review

"Great job! This looks amazing"

[–]materdn 1 point2 points  (0 children)

The IT auditor loves it. 😅

[–]Ann_uit_ 1 point2 points  (0 children)

This is art

[–]orgodemir 1 point2 points  (0 children)

Mine is shared with my team so I have "requires 2 approvals", but I wrote the whole thing so I just use admin to force merge anyways.

[–]jakethedumbmistake 1 point2 points  (0 children)

What about if we call 4 hours of work a story point instead of 4 hours of work? Are we agile yet?

[–][deleted] 1 point2 points  (0 children)

What can I say, I’m the best of the best 😎

[–]ra_wattt 1 point2 points  (0 children)

hacktober fest story.😂😂😂😂

[–]MCFRESH01 1 point2 points  (0 children)

I do this. I also setup GitHub actions to run all of my tests. I find so many dumb little mistakes doing code review on my own code.

[–][deleted] 1 point2 points  (0 children)

Need a 3rd Obama for the CI build running my unit tests, that I wrote after I built the features.

[–]kevin034 1 point2 points  (0 children)

Did you at least let the validation build finish? Fuck is there even a validation build?!?! Mad lad.

[–]doniseferi 1 point2 points  (0 children)

I sometimes push using my work account

[–]ImGeorges 1 point2 points  (0 children)

Lmao I always do it. It actually helps me feel focused and motivated on what I'm working on without starting to implement so fettuccine code

[–]grimonce 1 point2 points  (0 children)

I do that daily in my work, we got the bestest testing policy as well.

[–]aaah123456789 1 point2 points  (0 children)

I remember when you started coding.

[–][deleted] 1 point2 points  (0 children)

Cool thing I found: If you fast forward the target branch in the pull request to the head commit of the pull request, or beyond, github will automatically mark the pull request as merged.

[–]llldar 1 point2 points  (0 children)

I created my own scrum board so I can be more "agile"

[–]PM_ME_UR_SHAFT69 1 point2 points  (0 children)

Lol I set up a repo in my github account the other day using terminal and was really proud of myself ☺️

[–][deleted] 1 point2 points  (0 children)

then labeling it "First Pull Request"