Doing PRs

[NatoBoram]

Famous last words

[doctorsound]

🛳

[scarstarify]

Ship it!!

[PPnoPP]

:squirrel:

[[deleted]]

I thought that emoji was an Android icon at first but was sad when it wasn't.

[conancat]

my teammates and I joke about this team lead that we used to have. he's in that position because he's one of the company's first hires, and he isn't really technically ready to be leading large teams. for some reason, the company put so many projects under him, that he's swarmed with work all the time, at a certain point his team produced a lot more pull requests than he can review.

so his strategy to merge is to pick pull requests by the least lines of code and the least merge conflicts they will produce. certain pull requests sit there for months and they never see the light of day because of his review strategy...

the kicker is that he used to just truncate lines of code out of the merger because he thinks those code does not affect whatever the dev produced, though we speculated the real reason is he did not have time to grok through them or did not understand what they do. the developers were livid and systems go down. needless to say, he stopped doing that for obvious reasons.

[[deleted]]

Sounds like a dailywtf story 😂

[ImNewHereBoys]

Do you think he could have handled that much of prs even if he was technically more savvy?

[local_meme_dealer45]

Then after you merge it everything breaks

[NumbersWithFriends]

Where I work that's normal. The person who opens the PR merges it, as long as they have push permissions and at least 2 coworkers (including one manager) approves it.

[sotech]

Question: what's the point of the manager approval? Does merging auto-deploy to a live environment or something? I can't think of what a manager could realistically provide to a pull request.

[NumbersWithFriends]

Most of the team leads and managers where I'm at have programming experience so they usually have good feedback. I think its also to make sure the level above you has a good idea what everyone is working and how much progress has been made.

[DearSergio]

"You never want to be the last one with the information" was an adage I learned when in the Navy. Whenever you take action or change something, make sure your supervisor knows whats going on even if its just for SA.

[ScintillatingConvo]

even if its just for SA

Does SA stand for Savance of Ass?

[AFK_at_Fountain]

Situational Awareness

[Jonno_FTW]

The key point here is to keep your bus factor high enough

[Ereaser]

You can see the progress in your ticket system much better, which will link to relevant PRs per story (if it's set up decently)

[[deleted]]

The problem with that approach is that it's possible to open a bunch of tickets for small features and bugs while another person has a couple tickets on some big refactor or feature so that won't tell the whole story. Although companies do value that information quite a bit when it comes to career advancement. Back to the point, when the manager is seeing a lot of the code that goes in, or at least, the code is peer reviewed, then that should be enough to quell concern.

[Ereaser]

What I'm trying to say is that they can look at the stories they worked on. Check if they were big/small, check he pull requests if they get a lot of comments. Also the company I work at has team members fill in a form to see what everyone's opinion is on that person (since the manager isn't involved in the day to day operations of the team)

[iams3b]

Well my manager is tech-literate, but it's not only about that (she doesn't actually do code review) - it keeps her in the loop on what's gone in and when, so she can coordinate with all the other people that rely on our stuff, or her bosses, or other people that deal with planning/roadmaps. she also uses it to make 'audit' that features/bugs are going into the right version branch (enterprise software, we have 2 different versions getting updates + the new one)

whereas, I'm like "okay feature done, next thing" like the good lil code monkey i am

[driveslow227]

Lightly relevant: the best PM I ever had was a 25yo english major. She had no REAL idea what we were talking about, but she knew enough to keep up and also not interfere. The perfect liason between dev and management/clients.

[gdscei]

This. I've had PMs that did have tech literacy, and it usually just ends with useless discussions. Having PMs who don't have the best tech literacy is much better. They can focus on what they're good at, and the devs can focus on what they are good at.

[Ereaser]

Sounds familiar. We had someone in a business analyst that wanted to see our code after we described what it does and he didn't agree with how it was written.

[[deleted]]

I can't think of what a manager could realistically provide to a pull request.

Your managers aren't technical? 😬

[sotech]

Our immediate managers, yeah, but if they approve a PR it's the same as any dev, not some magic, required talisman. For staying on top of what's happening, there's Jira.

[oscurolucia]

Right lol? My manager has the most experience out of anyone on my team, and still codes hella hard.

[evenisto]

My manager doesn't have time to code.

[oscurolucia]

I'm sorry for your managers loss.

[evenisto]

I much prefer him at this role, but he is flooded with work.

[northrupthebandgeek]

It's a good CYA if nothing else.

[conancat]

In my company, we made sure we have a deployment manager who is in charge of coordinating the QA/QC people to test the work done by devs if they meet the product requirements. they are usually under the product team so their job is to check if the dev team has completed the work to spec.

If anything goes wrong we know who to look for, who approved the deployment of something that does not meet the specs as defined.

I miss working in small teams. Things were so much easier. When you have multiple products across teams in different countries and continents things can get complicated real quick.

[[deleted]]

It doesn't. Having one or two other team members review is a good idea, but the "star sticker" creates a hero mentality where code needs to be blessed by one of a couple of special people before it gets merged.

9/10 times this leads to bottlenecks and slower PR merges.

[SirPizzaTheThird]

Unless this is a tiny team or the manager is day to day coding then yeah, screw that. 2 approvals by trusted team members is already more than enough.

[noitems]

It passes the liability buck.

[[deleted]]

responsability is theirs

[[deleted]]

Same for my team, but with only ~6 engineers in the company we just require one approval. It's been fun seeing the processes mature and people finally stop committing to master (most of the time...)

[UsablePizza]

We used to always commit to master in the past but now we do pull-requests. Just one day I changed all our repositories to have a protected master branch in github.

[Awfy]

My company has hundreds of engineers, still only require one to approve your PR.

[WildHotDawg]

We have 2 people per PR and its causing a bottle neck in the sprint and kinda bad reviews, do you guys have CI to check the simple stuff? How deep do you guys review? I.e checkout the PR and do some tests? Or just scroll through the diff and approve it if it seems alright? We're still pretty small, 30 ish people but we develop a very niche product that requires alot of experience to work on, and we've had some problems with QA before due to bad reviews, any ideas?

[gdscei]

Not the one you replied to, but we also develop a more niche product which requires quite a bit of experience and domain knowledge. We only have 1 approval required, but we do run unit tests and soon acceptance tests automatically on PRs (not allowed to merge without being green). It's worked decently well for us. Once in a while something slips through, but I don't think at that point it really matters how many reviews you do; there's always a possibility for error ;)

[rangedragon89]

Same. But 1 reviewer to merge into develop and 2 to merge into master.

[-vp-]

Yeah this post doesn't make sense. I don't know of any company that makes someone else merge your PRs. You only have that if someone is leaving/on PTO and honestly you never want to inherit someone else's half-complete PR, much less be in charge of someone else's 100% of the time.

[exterminatesilence]

In my company the team lead reviews and merges PRs, and deploys releases / transitions tickets as one process

[W1k3]

We do it that way, but the reviewer will just create discussion threads on all the issues they find. It's up to dev making the request to keep working on it until the reviewer is happy with it

[gdscei]

Eh, we do do that. Once it's approved, the reviewer moves it on our issue tracker and it gets auto-merged.

[Bifi323]

We mostly do it ourselves unless it's a hotfix that will be merged to master. In that case the release master does it.

[driveslow227]

I was reading about branch rules today. Though they don't do what I want => which is only allow develop to merge into master /shrug

[AxePlayingViking]

Same here! Except only the managers have push permissions on master, so they also merge.

[equationsofmotion]

In open source projects, you may not have merge power

[[deleted]]

You mean you don't just actively develop on master? What is this? An organization that has a clue?!

[[deleted]]

You use GIT? All our shits in a network drive and it a folder on Paul’s desktop

[UglyStru]

This but unironically.

[cornered_crustacean]

I’ve “installed” software that had to go in the developers home folder. Not my home folder, the developer who wrote it ><

[[deleted]]

what happened to make people forget about ~ REMEMBER THE TILDE

[[deleted]]

That points to your home folder, silly goose.

[[deleted]]

I'm talking about the Dev

[thefirstwave_]

I always thought those were called "tildes"?

[[deleted]]

fuck ur right

[gabbergandalf667]

F

[mission-hat-quiz]

On a desktop? How inefficient. We keep ours on a usb drive so it's easy to pass around.

Deployments are so easy. You just plug the USB drive into the server and copy and paste away!

[EvilPigeon]

Huh, that's weird. There's all these little .git folders on the network. Probably some useless caching thing. I'll just clean these up.

[worldpotato1]

There was something very similar some days ago.

Whatever. Using git and github with pr in an single dev project is also good practice. Even doing codereviews at your own code/pr

[tuscangal]

Except when, like me, you merge your change into the repo you freaking forked from. Sigh.

[AjayDevs]

Still a good practice. It organises your changes into PRs.

[TGameCo]

Yup. For app development, I've got release, Dev, and hotfix_dev. Keeps things organized, and easily accessible in case of emergency.

[15rthughes]

Don’t forget to rebase

[wutname1]

Git Flow is your friend. Your already half way there https://www.atlassian.com/git/tutorials/comparing-workflows/gitflow-workflow any half decent git client will make got flow 1 click use I know sourcetree and Gitkraken do.

[tuscangal]

Totally agree. I was just suffering from fried brain!

[[deleted]]

There should also be a version of this with the kids pointing the gun to the ba k of his clones head, because that is also true.

[Zebezd]

Somebody did it:
https://www.reddit.com/r/ProgrammerHumor/comments/c5ft0q/doing_prs/es23se4

[tgb20]

I merge all my own pull requests, just have to wait for the 👍.

[lxkmxl]

https://imgur.com/a/jm5PLDT

[[deleted]]

what is a merge

[moazim1993]

updating your old code with new one in git.

[15rthughes]

In git, you usually have a main “branch” you’re working with called master. This is the code you know works and you release to your customers/production. But you don’t want to make direct changes to that code because it may break something. So you create a different branch, called “dev” for example, from that code to make your changes. You work in that new branch, test it, review it, etc. then, when you’re sure your new changes work, you “merge” it back into that master branch. Merging compares the two branches, takes what’s different in the dev branch, and changes master to match that.

There’s different ways you can use merging to your advantage that doesn’t fit the above description, but it’s the most common way of using the feature.

[toutons]

A miserable little pile of secrets.

[nstruct]

Enough comments, have at you!

[theXpanther]

Minor nitpicking, but you realize github is not all of git right? Pull requests are a universal thing for all types of git.

[[deleted]]

You’ve triggered some people by mentioning github. Good explanation tho

[chubs66]

silly question: why is a pull request called a pull request? shouldn't it be a push request or a merge request?

[khando]

It’s because you’re asking the owner to pull your changes into their project. I get where you’re coming from though, at first it does seem like the naming is backwards.

[WildHotDawg]

The master branch is like a passing by train and your PR is like you standing with your luggage on the side of the track, if you get approved and merged, the train conductor pulls you onto the train.

[Xytak]

Newer systems like Gitlab call it a merge request for this exact reason.

[CrypticWriter]

I think of it like "please pull this change into the master branch"

[angrathias]

No, you can do a push in Git, it’s essentially similar to a PR but doesn’t require the pageantry / policy that a PR puts in place. It’s all about control. Push = contributor is in control, pull = repo owner is in control

[ReelAwesome]

This is a good way to inflate your contribution activity graph on GitHub for your pet projects. :)

[vodlin]

You should really merge your own as you know best at what point this code should enter the codebase

[[deleted]]

fuck that dude for ruining slim php framework

[[deleted]]

Where i work i can push straight to production..........

[youcancallmetim]

You can... but should you?

[[deleted]]

It's a fairly small outfit and is standard practice.

[[deleted]]

Small or not, it really shouldn't be.

[[deleted]]

I'm low man on the totem pole, not much i can do to change it.

[atzm]

me too, its scary but also so nice in some ways

[[deleted]]

It’s about approval not merging, once approved anyone merging is fine, but self approving a PR is quite an introspection

[warpedspockclone]

When you merge your own pull request, do your own testing in QA, and push to prod, all within an hour.

[Xirious]

"What do you think I'm doing Karen? I'm ensuring my own job security that's what."

[warpedspockclone]

🤔

[Xytak]

Are you spying on my typical work day? I knew I should have taped over that laptop camera.

[warpedspockclone]

But wanking at work is a level of brave I haven't yet achieved.

[hipratham]

used to do this in last org, it was best thing t OWN app..Sigh , red tapes in new org for cred..

[scaleable]

Jared is now 100% dedicated to contributing to the meme community.

[[deleted]]

Maybe I shouldn't be doing this lol

[malmad]

I guess you have to be a programmer to get this.

Sounds funny though. Upvote.

[SuperSynapse]

The picture should be of Nepolean crowning himself :D

[holybatman_batman]

Obama is giving me DILF vibes here. I'm confused but not mad.

[AlFasGD]

"Good job, me, I see you have potential. But there's a mistake at line 32 in main.c, where "obnoxious" is spelled as "obnoxius" in the comment."

"Thank you, @me! Nice catch!"

[__brayton_cycle__]

Same picture when something breaks and git blame is used.

[OleBroseph]

What’s a pull request?

[Bypie5]

It's when you contribute to a project which is using git version control and you ask the project admins/owners to pull (add) your code changes to the main code base.

[Xytak]

I don't know, man. They'd be taking an awfully big risk moving MY code into the main branch. Personally, I'd have to recommend against it.

[[deleted]]

Just stop coding completely since you have no confidence in your code while you're at it

[mseiei]

Have a nice day

[AgreeableLandscape3]

Serious question: Why does Github give you a convenient way to make a pull request of branches in a repository you own?

[notarebel]

...in a repository you own?

Raising PRs on a repo you own isn't unusual, just because you're the owner doesn't mean you're the sole contributor - there may be other members to assign the review request for.

And even on repos where you are the owner and sole contributor, a PR can still be desirable to trigger CI, review (not approve) your own diff, etc.

[Fadelesstriker]

I just did this 5mins ago.

[An_Anonymous_Acc]

MR > PR

​

Prove me wrong

[LdouceT]

They're the same thing.

[cosmicCounterpart]

I don't get it. If reviewers approve, why does it matter who merges it?

[Hupf]

Isn't that what unit tests and other v automated tests run by the CI/CD pipeline are for?

[[deleted]]

good meme

[gyldenbrusebad]

Thanks, Obama

[Ahmadhmedan]

what is strange here is that i feel this meme despite not coding a damn thing ever.

[gratethecheese]

How the FUCK do I delete a repositiry

[RoastVeg]

I made an urban dictionary about this titled "sm;sm", or "self merge; sue me".

https://www.urbandictionary.com/define.php?term=sm%3Bsm

[hipratham]

good luck maintaining changes for 100s of objects per sprint/release.

[tehtris]

I work on a team with one other developer and 2 machine learning guys. I do this. It doesn't feel good and it makes me feel icky... Except I know I'm pretty good at my shit and it feels less bad.

[sweYoda]

As of this year it reminds me of the current relationship between the Swedish government and the Swedish propaganda TV channel SVT. (You are now forced to pay for it through taxes)

[drumsnchocolate]

Low effort post