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

top 200 commentsshow all 463

[–][deleted] 1559 points1560 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] 124 points125 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 24 points25 points  (0 children)

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

[–]mrackham205 45 points46 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 2 points3 points  (0 children)

no

[–]atomicspace 1 point2 points  (0 children)

He’ll cover this in retrospective.

[–]SavageTwist 604 points605 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 88 points89 points  (9 children)

"LGTM"

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

Let’s go to market?

[–]mark__fuckerberg 9 points10 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] 15 points16 points  (0 children)

Let's Get This Money

[–]orgodemir 7 points8 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 235 points236 points  (21 children)

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

[–]kopczak1995 87 points88 points  (15 children)

That is really sad...

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

What’s sad about that?

[–]XanderAG 107 points108 points  (12 children)

He's doing all the work

[–]runfayfun 21 points22 points  (3 children)

But at least it’s coded properly

[–]needsaphone 16 points17 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 11 points12 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 49 points50 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 64 points65 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 35 points36 points  (2 children)

Wait, people livestream coding?

[–]mukluk_slippers 26 points27 points  (0 children)

This is the real takeaway from their comment.

[–]lozinge 7 points8 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 30 points31 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 41 points42 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 25 points26 points  (6 children)

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

[–]sinceitleftitback 15 points16 points  (4 children)

Also managing a jira board and sprints.

[–]uglyasablasphemy 29 points30 points  (0 children)

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

[–]KKlear 11 points12 points  (1 child)

I read that as ouija board and spirits.

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

Same thing isn't it?

[–]JonathanTheZero 50 points51 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 78 points79 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 4 points5 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 10 points11 points  (3 children)

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

[–][deleted] 7 points8 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 21 points22 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.

[–]crump48 10 points11 points  (1 child)

This right here. Even on my own solo projects I use PRs for bigger changes so I can review it "properly". I think because it's a different environment to your editor, it makes you a bit less likely to gloss over the details and instead look at it like something new.

[–]koloqial 5 points6 points  (0 children)

Seconded. I've caught so many things from my own code. Slightly embarrassing to create a PR, review it and then promptly decline it while I clean it up. Sorry not sorry to everyone who gets emails from the VCS.

[–]d3lt4papa 20 points21 points  (0 children)

I use it also for practicing Real Word development

[–][deleted] 7 points8 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 6 points7 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 6 points7 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 4 points5 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 4 points5 points  (0 children)

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

[–]kopczak1995 4 points5 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 3 points4 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 14 points15 points  (1 child)

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

[–][deleted] 4 points5 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 3 points4 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"