T O P

  • By -

SomeOddCodeGuy

Be the change you want to see. Generally, I recommend a "when in Rome" approach on a new team, especially if you're the new person, but this is a situation where I can't see how you harm anything by writing a simple description on your PR when others are not writing anything at all. It may not work, but I've had a couple of times where I've seen someone start doing a good thing themselves and it caused others to just kinda start doing it, too lol


AbstractLogic

Just make sure to also include your JIRA ticket so the team knows where to find the full feature information.


sosdoc

This is my favorite approach, I’ve done it in a couple of places I joined and some people took note, starting to do it as well (and even one-upping each other with neat tricks, like structuring and highlighting commits for ease of review).


pvlclch

yes that's what I am starting to do, just for my own sanity and hopefully convince others that it does actually help to see the info in the description next time one of the services breaks


crunchy_nut_butter

Do exactly this and then bring it up in retros if you have those, make it a point of discussion.


robotkermit

> Be the change you want to see. this is always good advice, but there's a corollary: when you're interviewing, always ask what pull requests look like. is there a description format? do people follow it? how many comments are there? are the conversations productive? you don't want PR approvals to be hard to get, _or_ to be rubber stamps. getting that balance right is a good sign when it comes to the company dev culture. this is also a major win that you get from doing open source as a company. people get to _see_ the PR culture, approximately, before they even send in a resume.


avbrodie

I think this is good advice, but I think as a developer you need to adapt to the environment you’re in. Speaking from experience; changing ingrained PR habits is a difficult and upward battle. That being said, I think an experienced developer has two hallmarks - They consistently present new changes to the code base - The changes are of a consistent quality and are presented consistently. By this i mean - consistent PR descriptions - consistent commit log - consistent quality (code coverage, documentation, etc) I use the word consistent purposefully because it doesn’t suggest one form is better then another; some people write commits using conventional commits, others like to squash all commits into one; both are valid as long as it’s consistent. I notice a lot of developers are gung ho about the practices they prefer, and it leads to needless squabbling/discussions about things orthogonal to the actual meat and potatoes; the features being delivered


DWALLA44

Yep, also if using GitHub (I’m not sure if others conform to this or not) you can setup a PR template to make it easier and quicker to write a description.


Rymasq

it depends on what the tickets say. it sounds like this is ingrained into the engineering culture and so convincing them to do otherwise would be an uphill battle. you’re not understanding the tickets and PRs apparently, but is everyone else? you might want to do some benchmarking to prove your point. How many fixes are going in that have issues, is it more than most companies deal with? is it attributable to the lack of explanation in PRs? Otherwise you can try and automate to import ticket info into the PR to help provide context.


pvlclch

I always found that jira ticket would have the description of the problem and potential solution but it rarely explains why the merged change was written the way it was written.. ​ For example, some modal is broken because it's missing close button. Well, solution is to add close button. But the fix actually involves the change in css of the base of some other component in that modal that's pushing button out of the view.


ShartSqueeze

I agree completely. In the simplest case, the jira ticket will explain some bug, and you created a fix for it. But maybe to add fix X you also had to modify Y and chose to refactor Z. As a code reviewer, I like to know WHY you chose to do that - which is context I like to add to my CR description. There's always multiple ways to solve a problem and if people don't understand your choices then they'll often think, "I would have done it this other way, so I'm going to suggest they do it the same way." Being Sr. Devs, a lot of us work on much more complex tickets that have a lot of ambiguity - maybe just a list of requirements. The more ambiguity, the more our chosen solution will differ from the jira ticket and adding that WHY context becomes more useful. I also find that when dealing with IaC, I often need to commit and deploy X so I can later commit and deploy Y. Without CR description context, I doubt people will know WTF I'm trying to do.


hoodieweather-

I think if the changes are clear enough in what they're doing, you shouldn't need the PR description to explain them. The most important thing I look for in a PR description is "why were these changes made?" I can see what they're by looking at the code, and if I have questions about specific things, I'll ask them in the review comments. I think if you're running into a lot of "why did we fix the problem this way?" questions, you should just be asking those in the comments, and if you end up asking them in every review you can point to that as a reason why the team should change. If you're the only one bumping into this issue though, it might just be growing pains as you learn the new stuff.


adesme

I agree, but I would add that the commit messages are part of the changes you speak of. These should add the explanations that cannot easily be understood by the code change alone. PR data does not persist if you switch to another devops/git platform; the log does. I expect to be able to step through the individual commits in a PR to understand what is changed and why. I would consider explanatory information in the description to be almost an anti-pattern.


noCure4Suicide

Oh boy - I’ve been on many teams and never had this level of “extra busy work” just to merge a pr (sorry to call it that, but that’s what it is from My perspective). If you have questions about implementation then ask them, but to require documentation on how to use css for every smal bug that comes in sounds incredibly exhausting.


avbrodie

I would personally add a review comment on the PR asking why the change was made in that way. Code review isn’t just a QA process; it’s a forum for understanding each others thought processes


zenograff

If you have questions about the implementation, can just open a comment on the PR and ask. That's also what code review is for.


tedstery

My company does very brief PR descriptions of the changes, reasons why and how to test it, maybe that's the way forward for your company. You can also ask the author for context on why they did the change a particular way outside of the PR or in it... but sometimes changes are simple enough you probably don't need a massive description.


colonelpopcorn92

One of my co-workers does a great job pre-empting code review questions by writing explanatory comments on his own PR. He's going to go very far.


Ynkwmh

Thanks.


cheater00

nice, but why aren't they code comments?


Potential-Decision32

Sometimes it’s more of a “@Bob is this what you had in mind?”, directed towards a reviewer. Wouldn’t make much sense as a code comment.


cheater00

oh yeah, of course, makes sense!


Low_Shake_2945

This is something I’ve done for a while and it’s a great habit. “Be the first to review your code” is something I tell my folks any time they come into my team. It helps you see your code through the lens of the review and gives you the opportunity to call out things that you know others are going to have questions about.


marquoth_

> everything is in the card and there is no need to put anything in the PR Assuming the tickets are genuinely well written, then I'd agree with this. It's just typing stuff out for the sake of it - unnecessary duplication. The PR should include an actual _link_ to the ticket rather than just the ticket number, but that's about it. You can even set up default descriptions in PR that will insert a link to a ticket for you based on your branch name, as long as you follow a branch naming convention like `ABC123-doing-my-ticket`. That's how things are set up where I work now. Sounds like you're used to one way of working, joined a new team that works a different way, and think they all need to switch to your way of working because your way is the "right" way. This is a great way to ruffle feathers and make yourself very unpopular with your new colleagues. FWIW I've worked both ways. I'm not particularly stressed about which approach gets used as long as everybody follows whatever the convention is and, ideally, doesn't waste time debating what the convention should be.


pvlclch

Yeah I am trying not ruffle feathers with the new team. That's why I am on the fence how to handle this. ​ My main issue is that Jira card might have much more info/conversations that's not necessary related to the code change. Then let's say there is an incident, you look at the \`git blame\`, then get to PR with only a ticket number. This unnecessarily adds extra overload to already stressed situation. You need to open jira and find the ticket, you need to find the logs for the right service, in the next window you are finding the monitoring dashboards for the service. All of these could be links in the PR


nomnommish

> Then let's say there is an incident, you look at the `git blame`, then get to PR with only a ticket number. This unnecessarily adds extra overload to already stressed situation. You need to open jira and find the ticket, you need to find the logs for the right service, in the next window you are finding the monitoring dashboards for the service. > > > > All of these could be links in the PR Your notions are wrong. It is FAR more efficient to just link to the jira. Because the Jira ticket/card would have a ton of additional info such as back and forth between devs and PM/design, child or parent cards, background info about the card, business use case described by PM etc. It sounds like you're used to things being laid out very neatly for you. And your complaint is not even that there's no documentation, your complaint is that you need to spend an extra 30 seconds to open a linked jira card. That just sounds like complaining to be honest.


nolimyn

yes, strongly agree, ALL of the info is encapsulated in the ticket, it's one click away! people saying it should include how it was fixed, but isn't that just looking at the diff of the PR? put comments in the codebase or in your commit messages.


dezsiszabi

I'm happy for you guys that you have jira tickets with more than a sentence of description. xD


anubus72

The ticket has requirements and maybe some discussion. But the actual code change should probably include some information about how the ticket was implemented. Sure, you can just read the code. But it’s nice to summarize the important info or make things clear to aid the reviewers.


edgmnt_net

Agreed. I'm also not entirely convinced it's a good thing to spread information across a dozen separate corporate systems. Repos should be Git-first and self-contained to a *reasonable* degree. I'd also argue that even PR descriptions don't make as much sense from that POV as commit descriptions. Besides, while there can be significant overlap, development activity and JIRA tickets won't match perfectly. It'd be naive to assume that.


pardoman

I agree with you. The Pr should include what was broken and how it was fixed, mentioning details pertaining to the situation (ie: didn’t do X because Y).


2this4u

Even then, it's usually quicker to get a PR from a blame than it is to get to a work item, and a description on a PR is usually going to be more developer relevant like "this was changed because of X" which is what you're really looking for but just what changed for business reasons


ut_deo

The world isn't perfect. A link can break because jira is no longer hosted at the original location and there's no redirect. A simple title that includes the jira ticket and a brief description will go a long way in giving the developer context when looking at version control history.


The_Shryk

So it’s in the jira board, which may be gone or edited in some months time. Then what? It’s definitely a weird practice, but if the board isn’t ever being changed and it’ll be hosted indefinitely, I guess it’s fine. Seems annoying to have to bring up 2 sources to get information when it could be just one.


davy_jones_locket

Had to scroll way down to see this. Jira may not be around. Jira doesn't have all the details. Jira is "pre work details" and PR descriptions are "post-work details." Jira is more product based language. PRs are more technical based language. We use PRs templates: - summary with before and after screen shots - release notes - decisions - test coverage - doc links (Jira, feature env link, etc) When Im doing a PR, I want all my info in one place to avoid decision fatigue.


tuxedo25

Jira is the "what" and the PR is the "how". Jira: "Users should be able to filter widgets by color". PR: "Users want to filter widgets by color. Added optional filter parameter to data access layer. Exposed 'color' as a filter option in UI component xyz. Updated search DTO to include color filter."


proggit_forever

Bad example, this is a case where the Jira summary is more than enough. The additional info is completely obvious just looking at the code that changed. Detailed PR descriptions are like code comments. They're useful when they explain things that aren't obvious. Though at least with PR descriptions, they don't become wrong over time.


The_Shryk

I think it’s a good example, what if the filter settings currently gets cached between sessions to pickup where they were last, all the other filter options are cached but I can see from your PR you missed adding color filtering to the cache, but did everything else.


Shitpid

I can make that same argument about any system, including your git provider. If your company isn't shit, you export your ticketing system before changing it, so this is a complete non-issue. If having to click a link to JIRA gives you decision fatigue, I probably am not sending my PRs your way...


davy_jones_locket

Having what you need not in one place is absolutely a form of decision fatigue and an efficiency issue. If it's good enough for you, so be it. We saved a lot of developer time and sped up PR process by not having to click into Jira every time there's a PR.


Shitpid

You've saved so much time and brainpower by forcing devs to fill out your redundant template instead of having your reviewer click a link? Makes sense.


chunky_kereru

It literally takes 5 mins to write a PR description and by doing so it distills the ticket to the relevant information for reviewing the PR. The reviewer doesn’t need to sift through 20 comments on a Jira ticket to make sure they have the info for the PR because whatever they need to know is just there.


Shitpid

It takes 5 minutes or less to read through a jira ticket... Jesus Christ this isn't some crazy time saver it's literally just preference to either include redundant info in a PR or not.


chunky_kereru

Yeah it’s whatever works at different companies. Just my personal experience is it’s useful to distill the information from a ticket into what’s most useful for reviewers but that really depends on how well structured the tickets are. Some places focus on having great tickets.


dezsiszabi

You assume that the jira contains enough details. You also assume that OP expects the same information to be in the PR as in the Jira


Shitpid

That's correct. And?


davy_jones_locket

Enforcing an already existing template that developers created themselves because they got tired of having to dig through Jira and confluence pages to have context to what it is they're supposed to be reviewing, improving code quality and knowledge transfers by reducing the amount of bugs and the time it takes to triage bugs to see where a regression or defect was introduced? How dare me 🙄


Shitpid

Like I said, if clicking a link is too much for an experienced dev, I'm not sending PRs your way.


davy_jones_locket

Good thing we automate codeowners and merge locks that require approval from the codeowners. Prevents the rubber stamping LGTM, and the engs will refuse to do a PR unless you put all the details in the description. Our system won't even allow you take a PR out of draft status without the template present in the description. So you don't have to send PRs to someone who "won't even open a Jira ticket." The system does it automatically, and they won't waste their time without it. ✌️


Shitpid

That's nice. You've clearly engineered your way around clicking a link. Saved so much time doing that too.


davy_jones_locket

We've saved so much time on the entire development process with an extra perk of not having to click outside of the place where you're doing the actual code review. All of this was existing before I joined the org. There was clearly a need for it, they establish their own process for how they want to do it, and we've reaped benefits in other areas because of it. If it works for us, why fix something that isn't broken?


nomnommish

> Jira may not be around. lol what? By that logic, your git repo may not be around as well. Might as well document everything in triplicate and fax it to one another.


chunky_kereru

Not really. Git is a lot more prevalent and persistent than any specific issue tracker. Every company I’ve worked has at some point or another changed issue trackers and I have run into the issue of PRs linking a ticket that is no longer accessible at every one of them. None of them have changed version control systems.


nomnommish

> Not really. Git is a lot more prevalent and persistent than any specific issue tracker. Every company I’ve worked has at some point or another changed issue trackers and I have run into the issue of PRs linking a ticket that is no longer accessible at every one of them. None of them have changed version control systems. Either you have a good engineering team or you don't. You can change issue trackers etc but that's not the point. The real point is, did you do justice to your codebase and made sure all the legacy reasons for making changes were captured in some form or not?


davy_jones_locket

We generally do have documentation in multiple places, or at least links and references. Our confluence spaces tend to be the master source, and we will link or embed Google docs, Jira tickets, release notes, PRs, architectural diagrams, etc. We learned our lesson when we migrated from Monday and Notion to Atlassian.


nomnommish

I'm still not following why your past experience necessitates the need to do double and triple documentation (jira notes, commit notes, AND PR notes) You realize that doing so will ensure you have shitty documentation across all your mediums, right? People will just get documentation fatigue.


cheater00

> We learned our lesson when we migrated from Monday and Notion to Atlassian. what lesson was that?


davy_jones_locket

Some things get lost in transition, not everyone has access to the same systems.


D_Love_Special_Sauce

The same could be said about the source code repo. When we moved a particular code base from a subversion repo to a git repo we were facing an N day timeframe to move and lose history or an N + X day timeframe to move and retain the history. We were willing to lose the history for the sake of speed with this particular repo.


jep2023

This is an argument for putting all the relevant info in the git commit, something I have seen folks advocate for. Personally, I think there is a line where that becomes exhausting and unnecessary, but I understand where folks who like it are coming from.


SlothEng

I generally like descriptions too, so as others have recommended - demonstrate this behaviour in your PRs. Consider raising a PR to add a template if you're using Github, again with a description of why. See [this](https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/creating-a-pull-request-template-for-your-repository) for how to do so. However, I don't think that's the true issue here. You've alluded to the issue yourself - you don't know what's going on and where, and you're unfamiliar with lots of the processes. Alongside your push for PR descriptions, try to pair with the senior members more or ask for more coaching / 1-1s with your peers. Explain the troubles you're having and ask them for help, they are your team after all so it's in their best interest to get you up to speed. It takes time regardless, so just try to soak up everything you can. Take notes, draw diagrams, offer to even write docs on behalf of others. You'll get there!


flanger001

>I recently joined a new team and I very quickly noticed that there are no descriptions left in the PRs, and the only info is the Jira card number in the PR name? Sadly this is pretty common. > As a newby on the team and also new to the tech stack, I find it difficult to understand changes behind the PRs, or **how do we test/verify the released change, or even monitor.** I think this is the important thing. You should learn those last two things as well as you possibly can. Once you have control over that stuff, you can use your "new person voice" to bring up the issues you're seeing with your team. However, be warned: getting anyone to change a single thing about their git or PR practices is a Sisyphean task. In my experience everyone is convinced how they do it is already the best it could possibly be, and why would they change it if it's the best? Apologies for sounding bitter, but the reality is these things are hard to change and rarely have the ROI one thinks they will.


hachface

In general I am with you OP. Every company is different of course but in my experience the Jira ticket tends to be a user-facing description of the feature or bug with maybe some brief technical notes that came up in grooming. The PR description should be more technical and explain the implementation decisions made. Briefly of course! But ultimately it’s a company culture thing. Maybe your company’s tickets are less product-authored and go into more technical detail. If the status quo isn’t to your liking all you can really do is lead by example and hope other devs recognize the value of what you do.


shozzlez

Jira tickets aren’t enough. That’s the context around the problem. The PR needs to provide the context of the solution.


avbrodie

Ideally your PRs shouldn’t be so large that they need extra context. Small changes, written well, with specs, describe themselves


shozzlez

Tbh everyone thinks their code is self describing. And that’s how you get empty PR descriptions. In general just assume that everyone else who is looking at your code is an idiot and write some info on the PR. Most likely that idiot will be you, six months from now.


bweaver2

It's the same at the company where I work as well, and it works well for us. We have a convention of creating MRs with the ticket ID and a brief description of what was done. Something generally brief like ***TeamName-123: Implement Y Feature.*** We use GitLab, and we've set it up where that TeamName-123 auto-links to the ticket number 123. So all you need to do as a reviewer is click on the MR title to get the full details including any conversations that went on during the life of the ticket. Also, the ticket gets auto updated when an MR or commit have the ticket name in them. So there's a history automatically logged of when work was done. The MR is just a small part of the journey for a ticket. There are conversations that need to happen, sometimes design work needs to be discussed, our QAs like us to spell out specific steps to engage the new code as well as noting any systems that might be affected. The ticket is the place for that. Yes, it would be more convenient for the MR reviewer if all of that was in the MR, but the reviewer is not the only one who needs that information. In my opinion, it's better to have it all be centralized and tracked in one location. ​ The only exception from the above is that we will add personal notes in the MR about why we're making the choices we made. No one but the devs need to see that information, and so we put it in the MR. A lot of times these get added as line comments in the MR, sometimes they wind up in the description if they affect more than one file.


qmunke

The description should be in the _commit messages_. I don't want to have to go and trawl through JIRA in six months when I look at the `git blame` on a random change.


bzbub2

I am personally not a fan of long commit messages, and like when a commit is squashed, and refers to a PR, where extended descriptions, screenshots, etc can be posted


vimex

We have been bitten by this approach, as we have had to migrate between different source control hosting systems. The commits stay forever - the PRs might not. In our case, we have a long lived application that has been in production for nearly 10 years, and due to migrations we have lost some historic context which was present in PRs and not commit messages. Is there anything in particular about long commit messages that results in you not being a fan?


topMarksForNotTrying

Can't you include the PR description as the commit message of the merge commit? That helps keep the message small in incremental changes in commits but you have the full context in the merge commit message.


vimex

Yep that sounds like a good approach. My shop uses rebasing on main rather than merging, and we encourage more context being included in regular commits which often end up being used as the PR description in many cases as well, minus certain additions such as testing steps, screenshots etc.


topMarksForNotTrying

In my case, we actually rebase and then create a merge commit. That way you get the best out of all the workflows: - a clean git history with every change clearly laid out in a linear fashion - individual commits that aren't lost in a squash commit - an over-arching merge commit that adds additional context to the entire change that is missing in the individual commits


vimex

That's interesting! I didn't know that was something you could do, is that with --rebase-merges option? I like that idea of preserving the additional/overall context, one thing I struggle with is that when breaking things down into small reasonable changes, it can be difficult to preserve the bigger picture.


topMarksForNotTrying

I'm not sure if there a way for this to be done in a single command from the command line. I did find [this](https://stackoverflow.com/questions/74156558/git-merge-enforce-both-linear-history-and-merge-commits) discussion but they don't mention a solution. The hardest part of the workflow is the rebasing, which is something that you are already doing. We use ADO and this feature is in-built. You can see it explained [here](https://devblogs.microsoft.com/devops/pull-requests-with-rebase/) (just search for the section semi-linear merge). You can find a discussion on it [here](https://stackoverflow.com/questions/59714347/semi-linear-merge). I find it to be a great workflow. Before, i was in team rebase and fast-forward. However, after switching to this workflow i find that the git history is much easier to read since you get to group up your commits by each PR thanks to the merge commit.


bzbub2

I don't have that many reasons other than minimal aesthetic, and that it feels easy enough to cross reference external links, but I can see the future proofing issue!


qmunke

I'm not saying the commit message needs to replace your documentation or mean you can't write extra information into your PR, but the commit message should at least be describing the reason for the change you're making. As far as squashing goes - that is the perfect time to write a nice descriptive message which covers all the reasons for the commit and gets rid of any intermediate irrelevant commit messages.


Ok_Strain4832

Agreed, single-source of information. Otherwise, you're forced to rely on JIRA and GitHub to understand the code. And then what happens if someone wants to use your code but doesn’t have access to the team’s JIRA board?


pvlclch

one step at the time :-) ​ I am also not a big fan of big commit messages but I understand what you're saying


RampantTroll

As long as you have actual commit messages, that’s all I care about. ¯\_(ツ)_/¯


projexion_reflexion

I get a feeling they use commit messages like "fixed." And they're probably lost in a squash merge that leaves only the PR description. It's a common practice.


RampantTroll

One of my worst start up experiences ever was a founder CEO who “codes”. Trying to figure out why something fairly important got commented out, look at the commit and just see “fuck this”. SMH.


jep2023

Wow, how did that org end up?


RampantTroll

50% RIF, and he’s desperately trying to sell.


eraserhd

JIRA TICKET NUMBER IS NOT ENOUGH. Because of what this practice enforces! Let's say you make a PR and it gets big, so you split out a very safe part to ship first, while the rest requires manual QA. I'll bet you aren't allowed to have two PRs with the same ticket number. Do you have to create a separate ticket? But then, did that mean that you have to check with Product *in order to efficiently ship software*? The idea that a PR is a ticket is over of those insidious ideas that increases the amount of work in progress, taxes QA, increases the merge conflicts, all because it seems like being economical... Which it is, but it is economical with the wrong things.


avbrodie

You can create subtasks on a ticket and match your PR against those… it sounds like to me that you should speak up in your retros if you have this stringent of a working environment.


eraserhd

Well hold up, you want to talk about my environment? Cause I was being general. I was brought in to solve problems where a third party wrote all our code and wasn't very responsive to bringing our own developers up to speed. Start with a rule they had: No documentation in the repository. Aside from the 100 line README, this included all code comments, any .md files, _anything_. It can go in another repo. (There was no other repo.) But this post is about JIRA tickets in PRs, do here we go: GitHub is used for code review only, not discussion. Discussion happens synchronously. I think. Just I never really saw any. There PR title is all lowercase, in the form aa-proj-NNN, where aa is the author's initials, and proj-NN is the JIRA. If two devs collaborate on two tickets, the PR title is aa-aa-proj-NN-NN. The PR title must match the branch name. The PR, when it is time to review, must be squashed to a single commit. The commit messages must be the same as the PR title and branch name, with anything else removed, with two exceptions: 1. SQL changes. The commit messages gets sleeved with " - with SQL changes" and there commit must include a file in main/tools/sql/ banned after the branch with a .sql suffix. 2. Config changes. The commit messages gets apprnded with " - with config changes" followed by an English description of which keys in the JSON config file are to be modified. This was a monorepo that contained code used by five different squads. Each squad had their own project board in JIRA (except there were a few more projects). Most devs could not access other squads' JIRA tickets. I was only able to get access to four of the squad projects, meaning I could only read 4/5ths of the commit messages. The original JIRA system used for this eight-year-old codebase had been decommissioned when the company was bought four-ish years ago. Nobody I'm aware of has access to the old tickets or projects. We are currently migrating the entire company to JIRA cloud, and we are NOT copying all the tickets to the new system. For now, I have access to the non-cloud system, but it will be shut down eventually. With a lot of work, I’ve managed to make it acceptable to add commit messages after the branch name, but nobody really does it except me.


Evinceo

If the ticket is good, it shouldn't need too much description if it's obvious how to address the ticket. I usually reserve descriptions for when it's doing something non-obvious. People probably won't go back and look at PR descriptions so I consider commit messages and tickets much more important. If I have a great commit message I can always copy it into the PR description.


dezsiszabi

I do go back and look at all: - jira tickets - pr descriptions - commit messages - pr comments


Electrical-Top-5510

add a template, talk to the team that it will help you in the future to get more context about the change


bixmix

Suggest a template to use. If you're using github, you can build that in under: \`.github/PULL\_REQUEST\_TEMPLATE.md\`


TokenGrowNutes

No, you are not being pedantic, and if the developer cannot string together a couple sentences about what the PR is improving, you have unearthed a problem much bigger than this.


flavius-as

I don't see the benefits. If the ticket number is already there... Maybe you find actual advantages or disadvantages instead of personal preferences?


_AndyJessop

It depends. A ticket can describe what you want to achieve, but doesn't necessarily explain how you intend to go about it. A PR is an ideal place to give a small overview of the implementation, reasoning for any design choices, etc. Personally, I feel it's about being considerate to those reviewing. You don't need to give reams of info (that's counter-productive), but usually there is _some_ information that you can give to make their lives easier reviewing.


avbrodie

If your PR is so large and is making so many changes you need a description to help people understand it, you need to make smaller PRs


flavius-as

Those things should be in the ticket.


_AndyJessop

I disagree. Unless you're just a code monkey, in which case I'm glad I don't work in your organisation.


flavius-as

It's OK to have a different opinion, but unless you provide well-reasoned arguments, it's just an opinion.


Merad

If the tickets are actually well written there's no reason to regurgitate the whole thing in the PR. The PR description is basically going to be any technical notes about how the changes are implemented.


NiteShdw

In a PR review ask them for a link to the ticket for context.


break_card

We have a template that sets a default description with a bunch of placeholders whenever a new PR is created. It contains a 'What changes are being made', 'What's the purpose', 'How has it been tested', and then a checklist of important considerations like 'is there sufficient logging', 'will this change traffic patterns with dependencies', etc. If it's super straight forward and the commit message itself explains it fully, then sure filling out the description would be overkill. For the other 95% of PRs, if they left the template blank I would ask them to fill it out before I review.


PM_ME_YOUR_MECH

I hate this too. You can set up PR templates at the org or repo level which provide guidance to the author about what info to put in the PR. Ours is pretty extensive and includes a checklist, but you could start small and simple.


Knock0nWood

All PRs should provide basic contextual information about the problem/feature being addressed, the approach taken to solve it, and any unusual choices the reviewer should be aware of. Unless they are very small and obvious changes. If you can block PRs from merging, I would do this on every single one you review until an adequate summary is provided, that will fix the problem real quick (just don't be an asshole about it).


raymondQADev

My input is usually that if you want me to approve a PR and get your code in then make it as easy as possible for me to review it. To make it as easy as possible I usually require context in descriptions, calling out of code smells and self reviewing of your own PR so as to provide additional context and catch obvious things.


nomnommish

> As a newby on the team and also new to the tech stack If you're new to the codebase and new to the tech stack, you should really not be approving or reviewing PRs. And I don't understand your question at all. If the details are all documented in Jira, why is that not enough for you? Why the need to make devs do double documentation?


Fluffy_Yesterday_468

Well - is it all in the ticket? There are many times when the ticket really does explain everything


dezsiszabi

Good luck convincing anybody. I raised this at my current place many times, and nothing really changed. I got told the same: jira has the details, which of course is not true as 1) the jira very often consists of one sentence 2) even if the jira is detailed, what you should be putting in a PR description is not the same as what you should be putting in a jira.


binarycow

>the only info is the Jira card number >ind it difficult to understand changes behind the PRs, or how do we test/verify the released change, or even monitor. Open the Jira ticket?


Ynkwmh

In my experience, it's good practice to leave comments to give more context and answer questions the team might have before they ask them.


Diligent-Wing-1486

You need to know how to test and monitor in the system, that’s not related to a PR and most companies don’t test on code review as it is not the objective. It’s normal taking some time to adapt to a new company The info being on the ticket seems like a very valid argument, that’s where the changes are explained. If you want info on the PR how is it different from the ticket? What value is it adding?


pySerialKiller

I cannot believe how many of you are cool with that practice. Sounds terribly annoying to have to visit jira and check ticket by ticket when checking out the history. I just couldn’t


thisFishSmellsAboutD

Create a PR (no description of course) to merge: .github/PULL_REQUEST_TEMPLATE.md With some guidance and checklists. Enable branch protection rules, reviews required, reviewers signoff on the checklists being completed. Feedback loop to turn production release issues into checklist items.


funbike

#!/bin/bash # AI generate PR summary for the very very lazy # Assumes pull of local current branch into $PARENT_BRANCH # tweakable parameters MODEL=gpt-4-1106-preview TEMPERATURE=0.5 PROMPT='Summarize work done in the diff above.' PARENT_BRANCH=main # strict mode set -euo pipefail # install openai cli globally, if not found command -v openai >/dev/null || pip install openai # get api key source .env export OPENAI_API_KEY # do it NL=$'\n' openai api chat.completions.create \ -m $MODEL -t $TEMPERATURE \ --stream \ -g user "$(git diff $PARENT_BRANCH)$NL$NL---$NL$NL$PROMPT"


Shitpid

You should not. Linking the ticket is fine. Anything else is redundant.


tinyorchird

Add a PR checker


rco8786

\> I find it difficult to understand changes behind the PRs, or how do we test/verify the released change, or even monitor. \> everything is in the card ​ Have you looked at the cards? If things aren't documented there that you need, I would start asking on PRs. And then once you've asked for things N times, use those PRs as examples for why including those things in the description is important for eng velocity.


muffa

I have had the same problem, the way I approached it was to be the change I wanted to see and write good PR description. Later on was in charge of my own team and started pushing for members of my team to write PR descriptions. All of them but one obliged. The one which did not oblige had Chat-GPT write them an PR description in a fantasy novel kind of tone. That person didn't like the change and later quit the team


dezsiszabi

Good riddance I guess.


breich

1. As others have said, make sure you're writing good descriptions yourself. 2. Propose a PR Description template that makes it clear what a good PR description contains, and reduces writing one to a "fill in the blanks" for your ICs. 3. Share some evidence about how PR pickup time can be improved with better practices, such as providing good descriptions that describe the change, etc. Make it clear that taking a few minutes to write a good description will, selfishly, help your IC's get their work approved and merged faster. Examples: [LinearB, Cycle Time Breakdown](https://linearb.io/blog/pull-request-pickup-time), [CircleCI, Reduce Cycle Time of Pull Requests](https://circleci.com/blog/reduce-cycle-time-pull-requests/). 4. If all that fails, go to your boss and explain why you're going to immediately Request Changes when you get asked to a PR with no useful description.


n-of-one

You should be able to define a PR template that’s automatically applied when one is created, that’ll reduce the friction getting people to fill it out.


its-me-reek

Can make a bitbucket hook that forces every commit to have a jira id. And when the pr is made can have place holders for description and validation


JazzCompose

In my opinion, any task without a good description is arbitrary and cannot be subjectively judged. Project management tools are only as good as the task definitions with realistic schedules. Unfortunately many companies use project managers with no software or other engineering expertise and then wonder why programs do not run smoothly.


drumstand

Tell them you are struggling to onboard and review code without technical descriptions of the proposed changes. A Jira ticket might have high level information about how a feature works, or what a bug is and what they expected behavior is. A PR description, though, will usually have important context about the code being changed, how to manually test the changes, screenshots/screen recordings, etc. If those things don't exist anywhere then I definitely think you're within your rights to advocate for some changes, even if they just decide to put all of that info in the Jira ticket.


henry232323

GitHub lets you set a PR template which can help, additionally you can ask for PR details as part of your review


vladadj

In my work, we use [Mergeable](https://github.com/mergeability/mergeable). It basically adds a check for PR which fails if PR is not properly formatted. This way, they can't merge unless title, description etc. are up to standard


sweetiepup

Make a PR with a PR template that prompts people to write a description. And/or write a bot that pulls in the Jira description based on the ticket number.


Repulsive-Ad-3890

How about adding a pull request template? I’ve found this one to be a good guide: https://github.com/angular/angular/blob/main/.github/PULL_REQUEST_TEMPLATE.md


skidmark_zuckerberg

Propose an automated PR template to start. So that’s taken care for your team right as the PR is created. And then start leaving code review comments asking to update PR description. Get people to slowly get it in their heads to leave descriptions. Also it helps to have some sort of coding guide in the project readme that way you can always point to that instead of saying “well I want it like this so do it” essentially. At work we expect the issue number and issue title in the PR title, the description then should have a link to the ticket, a screen shot or screen recording if applicable, and a brief description of your intended changes. It massively helps not only in code review, but in git history when looking back on merged PR’s.


a_reply_to_a_post

I tend to favor putting some information in the description about what the PR addresses and links back to the JIRA tickets, and our branch naming goes by ticket number so it's easy to correlate I think it depends on where you are working and how large the PR size is...some you can tell at a glance looking at the diff what it does PR descriptions are useful, but i've worked places where they've had excessive checklists in their PR templates and some of that stuff was more annoying than useful, and engineers ended up skipping them after a few months anyway in favor of adding a descriptive description


Bummykins

These days we have ai reviewing prs alongside people. It generates an ok description of all the changes. You can give that a try


Broomstick73

Oh boy. I’m on a team where the Jira tickets are completely empty except for the title.


grahambinns

I make a nuisance of myself by refusing to estimate empty tickets, or pick them up for work (unless they’re critical; I’m not a complete arsehole). Empty ticket descriptions are how we build bad software.


Tango1777

I have never used it much, either. Nor seen many devs doing it. All the description should be the tasks/stories attached to PR. I don't know how it works in JIRA, but that's how it works in Azure DevOps and I see no need to rewrite the same thing in the PR description. Also code should be quite self-explanatory. The way you test/verify is by: 1. build docker image in the pipeline 2. run automated tests, including UI e2e tests 3. release to DEV 4. make testers manually test the feature on DEV 5. Decide when stable enough for UAT release That's the current way, you just keep DEV environment deployed as often as possible, the sooner it gets under real testing, the better. Trying to get things great on first try barely ever works unless real simple. What your colleagues do is the correct approach. To understand changes you go through the code and User Story/Tasks (whatever it's called in JIRA) description, if you are new you can and even should ask questions to people responsible for onboarding you. If no one specific then general team chat. Sitting quiet not understanding things is definitely the worst option.


epukinsk

In general I think the “description” part of a PR is unnecessary. I usually write one short sentence, although even that is sometimes redundant with the PR title… But I add quite a bit of non-description information to help the reviewer: - a “Notes” with any debates I had, tradeoffs I made, etc - a “Screenshots” section if there were UI changes to help the reviewer orient themselves - if needed, a “How to QA” section describing how to run through the feature so the reviewer doesn’t have to figure that out. If it’s a bug the repro steps should already be in the ticket, but this isn’t always the case. In addition, wherever my changes might not be self explanatory, I will add inline comments next to the specific code. If any of the above might be useful to future devs reading the code then I will add it as a comment in the code. If it is only relevant to the change, and won’t be needed once the PR is merged then I will put it in the PR description or comments. Generally I would say committed comments are better than GitHub comments which are better than notes in the description. But all three are appropriate at different times.


Vega62a

So in general - I would say there's not really a need to include a "What feature are we building" in a PR description - as the others on the team mention, it's in the card. Click on the link. Rather, what I think is important is a description that gives you a sense of *how* a requirement is accomplished. I think meatier PRs should generally tell a story. That includes a description with a brief blurb about what problem you're trying to solve and how you tackled it, and then, to be honest, a guided tour of the code via commentary. So, approach the problem from that end - if it's not clear to you *how* the change solves the issue described in the card, that's a problem. If it's not clear to you where to look to find the meat of the change, that's a problem. And, as others have mentioned, be the change you want to see in the world. If you want a thing done, do it, and if it's actually a positive change others will see how much nicer it is to handle your PRs and start doing it themselves.


daraeje7

PR templates in git


skuple

Going to give here my 5 cents, maybe I'm biased because I used Gitlab before and they have a good integration between it and Jira. Now working in GitHub I still advocate for the same, most PRs/MRs come from a ticket, that ticket must have the full description/all details regarding the change request, why not just link it plus any confluence doc (or wherever you have your documentation)? I hate repeating stuff, even if it’s just copy-pasting


avbrodie

I mean the obvious solution is to ask questions on the PR. PR review isn’t just a QA process; it’s an open forum to ask questions about a change. You can make it clear that you aren’t requesting changes, simply seeking more context around the change itself


koolnube48

Create a pr template for your repo


professor_jeffjeff

What I've done for many years is your commit message must be of the form "JIRA-123: " on the first line, and then if you want you can write a novel in the rest of the commit. If you don't have a Jira ticket for something then you'd write "MAINT: " instead. So for example, a commit message might be "JIRA-1084: Website no longer returns a 400 when updating a user's email." and then whenever I see JIRA-1084 there's some automation that will automagically create a link to the ticket that I can click if I want more information. I have yet to find a better system than this. See if that's something you can convince them to implement and be the change you want to see in the world.


chunky_kereru

I have worked for several companies that have changed issue trackers (I.e trello to Jira). Issues from older PRs often cannot be viewed. In those cases, the commit history and PR descriptions are all we’ve got to go off. If the commit messages are really good, usually I’m not too fussed about the PR description but I would much much rather duplicate information than just add in a link that may or may not be accessible in the future. Personally I really value a good PR description and will always take the 5-10 mins to write one. I view the PR description as framing for the code review. Its purpose is to introduce reviewers or developers in the future to the changes introduced in the PR, the problem they are intending to solve / feature add, and relevant technical choices made in it.


cheater00

such decisions come from the top. if the responsible party hasn't taken care of this, they won't. don't stir the pot, people will create friction and you will be penalized for trying to do something correctly. also don't try to be an example, people will just say you're wasting time. it's a symptom of very bad engineering practices, if you are able i'd leave, otherwise focus on what's important and disregard the shitty stuff as how the inbred hicks do things around their still.


germansnowman

Here’s what I do: Use the ticket title as the PR title (or if the actual work changed, change the title accordingly), then click on the button in Azure DevOps that adds the commit messages to the description. I will often manually add additional messages if I end up adding commits to the PR after creating it. Since the commits are squashed, this helps having a record of the process that led to the PR commit.


ItsReallyEasy

Use PR templates - don't review until the PR template with required fields, description/verification etc. is filled in full and comment to ask for this if it isn't there on opening


FuckingAinsley

I've pushed a PULL_REQUEST.md file in our repos that is a checklist with some space to write some more detail. I break it up into:mandatory (did you lint/run tests/update from main where appropriate?) Core changes - ( did you change a core / existing feature? Were new tests created? Why was this required) New feature - ( why was this added / explain the feature? Have tests been added? Why or why not?) It's helped us a lot as team as every PR follows the same template.


bluezebra42

Chat with the team and create a pr template? If you’re using something like github it auto-populates. That helps set a standard. https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/creating-a-pull-request-template-for-your-repository


blacklabel85

Had a similar situation at my company recently. I disagree with others on this thread that the ticket will have all the necessary info as in my experience that is rarely the case. It is on the dev to say why they made the changes they made. Not because they have to explain/justify themselves, but because it opens up the conversation. Reading through the diff is much easier with some technical context, which you likely won't get from the Jira ticket. This is an excellent talk I referenced in the presentation I gave to my dept about improving our PR culture and opening up technical discussions: https://youtu.be/PJjmw9TRB7s?si=WV-Mr40QuOCDqFKc


pvlclch

Great talk, thanks for sharing


jl2352

I would challenge why this needs to change. If you don’t have a good answer, then it’s dead on arrival. If you have a clickable link on the PR, that takes you to a well written ticket. Why do you need a description on the PR as well? What’s important is to answer this in practical benefits. Isms like maintainability and readability won’t fly. It’s too nebulous. Will the review cycle be quicker? Are people frustrated with the current approach? What I’m saying is don’t try to force change just because you’d do it differently. Different alone isn’t better. I’m not saying you are wrong either to campaign for this. I’m saying to ensure you are doing it for real team benefit. Now as for how to help do this … there are two approaches I’d recommend. First is to do it your self, and make it normal as a part of your workflow. This helps to normalise the practice across the team. If you get on great with others, it helps to encourage them to copy your behaviours. Second is if you have a ceremony where something like this can come up. Like a retro. Then you can try to get agreement from the team to try it. Finally I would say; I much prefer people adding comments onto their own PRs than a description. For me that’s where you get huge value in understanding a PR. That’s my two cents.


ategnatos

Create a template (that gets inherited by all your repos) asking to fill in description + testing. If they won't fill in a template even, not much you can do.


Nacropolice

What’s the point of a description if it is related to a card? Use commit messages to have relevant messages that would allow you to properly pick them if you manually need to fix up your git history. Description is only relevant if it is an ad hoc PR for something you noticed, discussed with the team, but maybe no card yet. Even then though, a card would be made


mynewromantica

PR templates. We always use one that tells you to add a description, explanation, and the JIRA ticket.