GitLab GitLab Contribute 2021, 24 Jan 2022

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: Code Review Blueprint - GitLab Contribute 2021

Description

Speakers: David O'Regan (Engineering Manager, Create:Editor), Jannik Lehmann (Frontend Engineer, Secure)

Slides: https://docs.google.com/presentation/d/1AuPIyDkpozb0nx2TZHIcEfIRo_0o1uoAd4RVr2RHUvU

In this session, we will discuss a Blueprint to a good Code Review. You'll be learning the most efficient code review patterns, how to best communicate with authors, how to use the GitLab interface and more.

00:00 Intro
02:11 How can I be a good author?
04:35 Reviewing a Merge Request
11:09 Receiving Feedback
12:23 Code Review Sins
16:55 Real World Examples
30:15 Q&A

A

Welcome to the talk on the code review, blueprint or formerly known as better code reviews, so this talk is being given by myself. At least I think so. I'm david an engineering manager over on the create editor stage and we have yannick.

B

Yeah, hello, everybody and thanks for making this to this workshop, my name is janik. I'm a front-end engineer on the security here together.

A

So we're excited to talk about some code reviews. So look you know a lot of people they've already been through the chops on code reviews. You might be asking yourself what have I got to learn here? Really, you know so we're going to tell you what we hope. You'll come away with at the end of this talk.

B

Absolutely that is probably a question that is going through a lot of minds at this workshop at this moment, but here's what we have for you if you're already a code when you visit you're, going to learn about technical empathy about how to beat you and how to collaborate like pro. If this topic is rather new to you when you're a new kid on the block, we'll make it better when it comes to the most efficient code review patterns, how to communicate with horses and how to use gitlab interface.

A

I don't know about anyone else, but I am sold so one forward that we want to mention something we pulled out of our code review values. Much like the way we have our credit values is when.

B

A

About code, reviewing the thing at the forefront of everybody's mind should always be merge. Profitability. What is merge profitability? It is essentially a percentage representation of whether or not the merge you are looking at is gonna, add value to the code base and that value can either be for developers.

B

A

Be for users, but essentially we're always aiming for a higher percentage of merge profitability and that's what the code review process should really drive. You've left the link in this slides to the reviewer values. If.

B

A

To read more about this asynchronously, but this is the core keystone to keep in mind when reviewing code.

B

Great stuff, let me take a second and tell you: how are we going to do that we'll be having all the details about what makes that code good or bad? We actually prepared a real world example and give that to the thoughts and we're happy to answer all your questions on the topic in your in our q, a after the session or asim um in the issue which you're going to find in the chat.

A

So before we jump straight into being an awesome reviewer we're going to talk a little bit about being an awesome author. You know, because if you're going.

B

A

Code you've got to write some code or, if you're, going to review changes, you've got to write some changes. So what are some things? We can do to be good authors? Well, the first thing you can do is whenever you're offering a merge request. You can put yourself in the reviewer's seat ahead of time and you can self review your changes in the exact same way. You would review any other merge.

A

This is an invaluable asset and it can really really help save time and reduce feedback cycles. In your code review.

B

Process, if you.

A

Know something is complex if you know it needs extra context. Maybe you just didn't quite get to that last thing and you know you're going to be asked for a follow-up go ahead and create it acknowledge it. Your reviewer will thank you and you'll save time.

A

The next thing is to look at always showing empathy in your communication make sure that you are empathetic in the way that you approach your communication with your reviewer they've taken the time.

B

A

Invest and sit down and review your changes and so make sure that your communication comes from an empathetic place.

B

Absolutely great stuff, before you hand dmr over, please read your own description and seminar check it. You want to focus all the concentration. Your very has on make it easy for them to understand the problem you're trying to solve this also comes along with providing all the information and banners in a digestible way. Your revenues will surely appreciate that. Do we have anything else to consider today.

A

The last thing to consider is a pattern we would like to encourage within your code reviews so in gitlab we're not opposed to squashing commits and rewriting history. If it needs to be done, it needs to be done, but a valuable pattern to get into the habit of if you're using it is to commit small and commit often and to use at looking small commits to build out a linear change history and emerge. This gives the code reviewer a set of contextual steps committed by commit.

A

They can step through to really gain context for how your merge came to be and what they should really look at in a review, and then the reviewer can always squash the commits at the end of the commit to our standards. But this.

B

Is a really really fantastic.

A

Tool to get in the habit of especially if you're, dealing with very large contextual emerges and changes.

B

Absolutely yeah, so let's switch sizes and speak about some best practices when you're actually reviewing the merch request from somebody at gitlab or someone from our community assume positive intent. Nobody will assign you a not perfect, mr just to annoy you. There might be reasons for it that you don't know about so lean back, be relaxed, provide feedback in a timely manner if a merchandise is out of your comfort zone or you currently don't have any time to revenue, it there's zero. Shame in saying that and assigning it to another person.

B

If you're doing the review, please make sure to provide some feedback within two days when it comes to giving feedback, please be sure to be as precise as possible. Don't let the alpha assume what you might have meant to say. Clearly the most important thing to me: don't let each other fail if you as a regular notice that the author is having a hard time solving the problem, help them out, try to point them in the right direction.

B

All of these four bullet points are sub values from recorded relation, while you're here, since everything starts with a nurse request around you, you couldn't have said there was anybody.

A

Fantastic, I think that last point is my favorite, especially when we're keeping merge profitability in mind. We're here.

B

A

Each other not terry to the down, so what else can we do when we're reviewing a merge? Well, we can actually make sure we understand the intention of the empire, and this is something that can be done through looking at associated issues associated ethics, maybe even getting in touch with the product manager. Who is driving the future that spawn the mr? We need to really understand. What's the intention of the mr, what the problem is being solved to really be able to justify good review.

A

So if you are not sure, if you find yourself lacking context or what the mr is trying to solve, it's gray in any way feel free to reach out to people go through the ask: look through associated issues, gaining context and understanding the problem. That's being solved is key.

A

So another thing to ask yourself is: is the author of this merger press working in their field of expertise in gitlab.

B

A

Working across stages, working across technologies working across problems, and often we have authors who are working outside of their level of comfort- you might be a front-end developer and you've decided you want to try a bit of full stack and you've offered up some rails and or spec.

A

It could be your first time and so as a coder viewer, it's always good to keep an awareness of where the author is and their technical proficiency when they're offering a change, and if you find that they're not quite where they should be work with them, look at as an opportunity for upskilling and for some serious collaboration last.

A

Does the author need additional explanation of the problems at hand? This, thankfully, does not happen too much, but as a good coder viewer, it's something to watch out for, and this is something that specifically targets maintainers.

A

You do need to make sure that the author was solving the problem that the issue or epic actually described. So you need to make sure that they're in alignment with what their pm or what ux is driving from the change. If it looks like it might be in some way out of sync feel free to reach out to the pm or ux directly on the merge, this creates good collaboration, open transparency.

A

We encourage that everyone is aiming for the right goal, which is to create lovable features for our users.

B

Absolutely, but there are even more things to consider giving praise is free of charge. So if you see something you like just say, it be careful with phrases just for the sake of phrasing. Them encourage errors and analyze them. Make pmr of psychological, safe place. Emerge request is a place of human interaction, and humans tend to make mistakes. If that wasn't the case, we could skip our revenue process completely celebrate errors that are fine, because that's a great thing when a bug was catched before it even hit our main crunch.

B

Another thing to keep in mind is to define goals and make clear what's needed for approval. This is crucial, especially on larger requests. As an author, it's sometimes tricky to find out what is good advice from a regular or an actual issue, so provide additional information about whether your comment is.

A

Fantastic and encouraging, as psychological, safe space and merge requests is really really fundamental. In gitlab we work, especially with maybe even unconscious biases, such as authority bias. If we're looking to a maintainer.

B

A

An example, and so having people out in front leading creating psychological, safe spaces to fail to make mistakes. Experiments is a really.

B

A

Part of the process, so last but not least, some of these are so important. We've got to repeat them so review fully. Every time you sit down to review, don't let merges or code reviews develop a long tail. This is a poor.

B

A

To get into, if, for some reason, you are not able.

B

A

Your entire context and your entire attention to reviewing in once in one swing, it's okay to hand off it's. Okay, to raise your hand and say I'm busy. Maybe I've got other things going on, maybe there's something happening in my real life. Maybe I'm.

B

A

Pulled away from work, fine raise it. Let the author know give them a rough time estimate when you can get back and when you can legitimately contextually sink in and if that's not acceptable. Let the author know they can always hand off to somebody else.

A

This feeds into the next point, which is if, for some reason, you find your context or concentration being broken by lack of understanding or awareness feel free to ping people blue people in often blue people. In early, we don't believe in bothering people we have short toes here at hitlab is of value. So we, ideally you get experts in as soon as possible. If you're, something you don't understand,.

B

A

Want to continue the merge training next be precise in your language. um This is a sin that a lot of people suffer from when they're reviewing code. We need to be very deliberate and very nitty gritty in our language. Take the time to genuinely explain what might be the issue. What might be the problem?

A

What you're looking for from the author authors are not mind readers and we need to make sure that we express ourselves succinctly in our language last recommendation, which is going to be the basis of how we demonstrate the code review in the live example is use the conventional common system developed by our very own paul slaughter. The conventional common system is a wonderful contextual common system designed to inject your comments on a merge request overview with stateful context that an author compares. It also makes them grappable for later on.

A

If you need to pull them up for some reason, so we'll touch.

B

On this, a bit in.

A

B

Awesome yeah, so let's go full circle and speak about best practices when you're receiving feedback as an offer be fearless about making mistakes. We link to your great example here and we will share our slides um within all the shared contributed stacks later on and go check it out. It's great. If you keep in mind that assuming positive intent is a two-way street, it also applies. When you see my feedback, you lose the ego if ideas and your changes are being questioned, try to stay, open-minded and as objective as possible.

B

A code value is all about finding the best solution for a problem, and nothing like that last but not least, be proactive with collaboration. If there are any blockers coming up in your review, make sure that you ping people that can help out or open up follow-up issue issues ahead of time.

A

That is such an important point about losing you practice.

B

A

Loud for a reason, the end goal.

B

Of every merge.

A

Is increased, merge, profitability and that comes from working together. Individuals, don't ever really build things, it's teams and to work cohesively. As a team, you have to feel safe to fail.

B

To make mistakes.

A

And that's the attitude we want to encourage across our company, so a handful of code review sins before we move into the actual live demo, and these are things that, if you find yourself in the habit of doing in your code, reviews stop take some time and figure out how you can stop doing these on a you know a regular basis, if possible, the first thing to be poorly written or even worse, no comments to be honest, they're just as bad as one another, poorly.

B

Written comments it.

A

Speaks for itself, um for maybe whatever reason you haven't had the time to express yourself. You use ambiguous language. You don't lean into collaboration, look to suggest solutions or look to work with the author to find a better way of offering up some code. It's not a good way to review code, you're not going to feel good about it as a coder viewer and the author's not going to feel good reading.

A

It next no comments- and this is something that we need to be very cautious of if, for some reason you find yourself reaching for the hit approve button without having left any comments really take a moment and stop and think is that actually, what's supposed to happen as a rule as tom as soon as a merge request, is over 10 lines if you're hitting a proof regularly with no comments either the person you're reviewing is an absolute 10x 100x as an x developer or there might be something wrong in your review methodology.

A

So make sure you question yourself before you get approved. If you don't have anything to say it's not that we always have to have things to say, but we need to make sure that it's not a habit.

A

Nitpicks the books are a double-edged source and they can be both good and bad. So we need to be careful if we're using them in code reviews nitpicks, where they can be useful, is if you're suggesting something that might be succinctly beneficial to the code itself, so you're actually going to increase, merge profitability by offering this.

A

But if that is the case, we would actually encourage you to use something else, maybe like a suggestion which we'll demonstrate later from the conventional commons, syntax nitpicks have this terrible habit of becoming high cognitive load and they generally don't actually increase, merge profitability. So if you find.

B

Yourself, leaving nitpicks.

A

Because it's your personal style, maybe consider leaving no involved next, not locally tested or not testing locally. So this does come with a caveat if it's a one line change to some copy, you're, probably okay, to skip the local testing. However, as soon as a merge requires some sufficient complexity, you're really not going to be able to understand the full scope of the changes without actually testing it, and we have different.

B

A

This in gitlab, but keep in mind during a code review process. Some person in the chain eventually should review the code locally. It could be the initial.

B

A

It could be the maintainer, it could be ux, it doesn't necessarily matter who, but somebody should look at this locally to make sure we haven't introduced edge cases because, as.

B

Human beings, we're not good at spotting things.

A

Like that, just by reading lines of code, radio silence- and this might be the biggest sin on this list- you get it you're busy. Everybody is you work at a busy company things change things are always moving if you're being pinged and you're being asked for reviews- and you know- you're not going.

B

A

Get there instead of leaving them on radio silence, consider going into the merge and leaving a comment with the author to express, hey. Look. I see you pinged me, I'm busy. I might get this in the next day or two.

A

If that's okay, you can leave me assigned, if not feel free to assign it to a different reviewer, but also consider feel free to reassign it yourself if you're just not going to get there, it's much better to do that to respect the author's time than it is to let the merge sit because for the longer the merge sits, the quicker the code, rots more context is lost. So if there's one thing you can avoid out of this list, please avoid radio silence last, not understanding the code you're reviewing. This is a problem.

A

If you find yourself reviewing code that you don't understand, contextually reach out to domain experts and the domain expert in this case could be the author. It could be somebody who is an expert in the technology. Maybe you need a graphql expert. Maybe you need a rails expert. It doesn't necessarily matter just if you find yourself dealing with things, you don't understand and you can't dedicate the time to avoid the urge to hit approve, just because it's easier look to reach out to people for help. We work with a company.

A

This is why there's lots of developers get some help. That's okay, lean into collaboration, work with the author, so moving on from our code review, since we're gonna actually go through a live demo of what we hope makes a good code review and give you some skills to take into your next set of code reviews. So what we're.

B

A

Is I'm going to give a slight example and then we're going to jump over to a real world example with yannick bionic has given me a pretty kick-ass code review and so, when I hand off the yannick feel free to just click on his screen to magnify us.

A

So first context context is king and generally, when we talk about comments, we have two forms of context we have blocking, which is not we're going to have.

B

A

This before we can before we can merge this code. The second.

B

A

Of thing is non-blocking, the second type of context is non-blocking, which is this is worth noting, but it's not necessarily a show stopper. We can actually move ahead, but maybe this warrants a conversation.

A

First thing to talk about is merge, meta emerge meta, it doesn't get enough attention, but it's really important for good reviews. So, as a good author and as a good reviewer, you should always be practicing good, merge meta. This means good titles, good descriptions, good demos as a reviewer.

A

If you arrive in and you're not able to understand contextually what the change is supposed to do or what the merge is going to do from the description, title and description or from the title description of images, you're gonna, probably struggle unless they've written an amazing test suite, which the odds of those two things happen in conjunction are not super likely.

A

So what I'm gonna do is I'm gonna hand off to yannick who's, going to go ahead and show you what a good good merge method looks like.

B

Absolutely so, let's jump right in into the mr that they've asked me to review. Let's go for his initial description at first, it says tiny mvc for an issue where we migrate the merge frame when dots powerfully generated on the back and into the front end to avoid drops and redundant code. I don't know about you this. I don't really know what's going on so in my revenue that was actually my first point. I mentioned as a non-locking suggestion that this description was a little hard to get my head around. I helped him out.

B

I provided a screenshot that is yeah to update his description, to make it easier for a maintainer or everybody else, checking on this to actually see. What's going on, I provided a list on how to reproduce the behavior on a local machine and even a little git patch, to make it easier for everybody involved to figure out. What's going on.

A

Stuff, it was definitely a lot nicer than my first attempt anyway. So next, what we're going to do is we're going to talk about issues and keep in mind. All of these have come from the conventional common system, so in.

B

The slides itself, we've left the link to.

A

The conventional comments, api docs, so we encourage you to have a read through, but the meat and potatoes of the conventional common system is made up of two parts and the first one is issues. So issues are highlighting specific problems. Often they are blocking. If you find yourself reaching for an issue, this is more than likely a show stopper for this merge. What we want to do is we want to get in the habit of writing good contextual issues and really laser-like focus in on what the problem is using precise language. What we don't.

B

A

Is we don't want ambiguous language? We don't want to just leave a comment on an arbitrary line of code that says hey. This doesn't work. What we want to do is we want to say this is a problem. This is why, and then we want to look to collaboratively, find a solution for it. Yannick, thankfully, has found a blocking issue and he has found a way to work with me collaboratively for us. I'm going to hand off to him now sweet.

B

Thanks yeah, there was one issue: actually we found in uranus and you can see all of this in the demo video I provided, and that is one of those takeaways or a good example. I I'd like to recommend to everybody just pop up one of those videos. There is super fast to create and really make a difference when it comes to understanding the issue that you actually speak enough. So please feel free after this talk. Just give this a spin and yeah. Let's use.

A

B

As an example other than that, I describe the issue in a short text and try to make my best helping out fixing it by providing a suggestion to just reply. It was locally tested and when you use one of those suggestions, please make sure that you do the same.

A

So I particularly found the demo video here very helpful. So as an author, the first thing I did was, I watched the video and I saw the problem that this was causing. uh This was being caused locally and.

B

That was great because I provided all the context.

A

For where yana could highlight about so next we're going to move into questions and generally you're not going to see too many questions inside merges, but we would highly encourage you to use them very often. This is for breaking contact silos. So if you find yourself unsure of something that you are highlighting, what could possibly be a potential problem but you're just not quite sure, use a question and when we use questions again, we want to use good, precise language with our questions. We want.

B

A

Really laser like focus in on what the problem we think is, or whether or not it is a non-issue or a non-blogger, and so to give.

B

A good example of.

A

A question I'm going to give over to yannick because yana had a blocking question, which is even more interesting.

B

Absolutely, and so I did- and I can't even stretch this enough david here- are you speaking of an incredible complex code, basically here at kidnap and yeah, just very, very hard problems to serve and revenues as well. They will not know everything so with this question, and that's why I was just not quite sure, what's going on here and I've been yeah looking into um a thing that was removed and I'm seeing the potential of them that this might not be removed fully.

B

So that's why I asked- and I put this blocking beam like I see some issues coming up, because we don't want to deal with this later so yeah, it's not an issue. It was just I as a level, was unsure about and asked for help.

A

And that puts me as the author in a good position, so you've approached me with empathy with kindness, you're just highlighting something you're, not sure about it. Maybe it's something I myself as an author, didn't think of and just missed, and that can happen we're just humans.

A

So the second part of the meat of potatoes, of the conventional comment system is a tool called suggestions. So suggestions are where you're going to work collaboratively with the author to propose changes inside the merge.

A

We have two ways generally of proposing changes and our first is unit using our native gitlab ui tool, the suggestion tool, which is a really awesome tool, and I quite like it. I think it's fantastic for small succinct changes, but the trick with using the suggestion shield is just that as yannick had mentioned previously, if you're going to offer changes using the suggestion tool make sure that you have tested these changes before you offer them to the user, make sure that you're not offering changes across complex pieces of code.

A

It's good for one to three lines. Anything more. You might want to consider a patch file make sure that your formatting is correct if you're going.

B

A

A suggestion it's a great thing, it's a collaborative thing, but make sure you do the author, the service of getting it right yourself. So I'm going to.

B

Head over to yannick.

A

For what makes a good suggestion and remember, suggestions can also be non-blocking suggestions. Don't always incur a blocking comment.

B

Absolutely yeah on dave's merch request. There is actually a good example from another suggestion, so he's been working on some specs and those are actually running fine, and there is nothing really wrong about this. I just found a little thing to do better and to help us maintain this easier in the future.

B

So, therefore, this is absolutely not blocking dave's first version is working as expected and is testing all the right thing, but there's just if you're, considering taking this up a notch, then maybe consider accepting the suggestion.

A

And something specifically, I want to highlight in this suggestion here, which I think is fantastic tactical empathy is yannick. Thanking me for the test. Before we begin so yannick has acknowledged my efforts. He's acknowledged that I have tried to make the code base better.

B

A

Wants to work with me collaboratively: it's not just a shoot down of hey.

B

A

Can do this better, we're really encouraging that collaborative back and forth. So that's.

B

A great example of tactical.

A

End to be mixed with making a viable change to codebase the next way of making suggestions, uh if this is specifically targeted for complex suggestions that maybe span a couple of lines or a couple of files and that's a patch file specifically so a patch file is a generated file. That's going to come from the diff that you will create locally, and then you can attach that inside your merge request for the author to review and if you're happy with it, they can apply it.

A

So why use a patch file over pushing directly to somebody's branch? Not that there's anything wrong with pushing directly to somebody's branch, but what patch files do? Is they really lean into the collaborative spirit of hey I've tested your code? I've looked at it. I think this can be done slightly better. I'm going to give you this patch file that I wrote and you're going to.

A

Let me know we can go back and forth on it and really invites the collaborative spirit and they're really fantastic thing to see being used more and more by code reviewers as standard yannick.

B

Is going to show you.

A

An example of a great gift patch for my merge, absolutely.

B

Yeah we're going back to my very first comment and which was actually talking about the merch request later information on this. So I applied a git patch to make it easier for reviewers or everybody involved to reproduce the behavior of local machine. So git patches are such a great tool. They can be used on any kind of issue or comment you're actually going to make.

B

I highly recommend applying those if needed, to emerge because description as well just to make it sure we're all on the same page, so go ahead and catch all. This.

A

That's everything it's fantastic and again you can see that yannick has explained thoroughly why he's arrived at a git patch he's been very precise in his language for the problem it's attempting to solve, and that opens me the author up to good collaboration with him.

A

Last but not least, my favorite part of the conventional comment system is praise. Praise is a unique thing to the conventional common system. That is something I hope to see really.

B

A

Used across the lab code base because it really.

B

A

Foster a fantastic environment for the encode, the praise is where you're going to highlight something positive and the trick with praise is to really make sure that what you're praising is actually praiseworthy, you're, not making something else that can actually be detrimental to the author as an example, something I genuinely love when I see in reviews is, if somebody writes their test specs to succinctly describe what the feature is doing so. Essentially, I can start by reading the tests. I know exactly what this feature is going to do. That is something I love to see.

A

It's something I encourage, and so every time I see it, that's a big old phrase from me and I was going to show you an example of what an actual praise looks like in a merge using giphy and everything.

B

Absolutely yeah, I found something to be praiseworthy in your mind and it was it had to do with tests, though, so um after writing some tests removing some tests rather- and this is something which is easily forgotten- I'm just gonna we're gonna be left with a huge of cluttered mock data that nobody really knows about. What's actually going on, so it's important to keep those things yeah clean. So um that is absolutely great for me.

B

um I added a given there just to make it a little fun and show some appreciation on this, because that's good stuff over there.

A

Absolutely the two pieces of advice, I'll give you, if you're using praise the first piece of advice, is to start your review with praise, find a good thing first and go from there too often as humans. We tend to skew towards the.

B

A

And it can be very tempting as a reviewer to jump straight to problems in code. Try and humanize the problem by looking at the good things first and then second, the anik did involve a gif. Everybody loves a gif, it's a humanizing thing and it looks fantastic on merges and it creates a fun safe environment to you know, make mistakes. It's not.

B

All super serious.

A

All the time so that takes us everybody to the end of our presentation on better code reviews of the code review, blueprint or whatever you would like to call it. We don't really mind. Does anybody have any questions we'll jump over to the q.

B

A

Panel and see if anybody.

B

Has any questions if anybody wants to join us feel free.

A

So yeah, what did you use to create the demo video.

B

And I'm actually using a software called obs which is open, source and free to download it is. It is able to do a lot, so it might be a little bit intimidating, but it's not too hard to get your head through. I definitely know there are plenty of other options. uh Quicktime player can do the same.

B

I know about some people actually using like recorded c sessions with themselves, so there are plenty of options, whatever works for you and I go by obs, which I'm going to put a link in the chat frequently and yeah one more thing, actually that I tend to do and if you are working with longer diamond videos, because gitlab has a maximum upload limit from 10 megabit, which can be quite annoying. What I tend to do is then still upload it in the resolution. Whatever does make sense and then just privately send whoever.

A

B

A better version for it, so they can constantly constantly do it.

A

That's a fantastic tip. I personally, I tend to just use quicktime and then trim the video down as needed and reduce the quality if it's too big, but I do like obs quite a bit. It's quite useful technology and, as annabelle mentioned people campaign for loom licenses in the future, so I would highly recommend swapping over.

A

You have any other questions. No, but kai has mentioned we're, making the conventional common system native we're thinking about it. I will be jumping on that epic, my friends.

B

That sounds absolutely fantastic: yeah yeah.

A

It's a really fantastic system. It's it's! It's the fact that you can inject context into every single comment. So, as an author, the biggest problem with code reviews is as an.

B

A

If you submit a piece of code and you go to bed and you wake up and all of a sudden, you see an email or a notification, and you have 20 comments to work. I can actually be very demoralizing over time and since this is our bread and butter, we do this a lot and take a really big toll on author. So breaking that into genuine context. So, as an author, I know exactly all right, I'm just being asked a question.

A

Okay, there's only really out of these 10 comments, one severe issue: everything else is, I can navigate us, makes for a much better experience.

A

B

Thank you very much.

A

For the link bs code for creating patches, there's.

B

A couple of links I'll.

A

Add to the bottom of the slide for creating patch files. um There's the one from the vs code. There's also one from ruby, mine and jeff rains, but even great, even getting comfortable. Creating them locally is a great skill to have.

A

Super stuff, I don't think we have any other questions. Does anybody else? Oh here we go austin when working with non-engineers. What are some things you've run into that helped or hindered the review process. That's a really good question.

A

Wanna. Take that one I'll.

B

Give it a go um I'd say actually all of the above like and definitely when reviewing things from one engineers they might be not so familiar with our practices and with the techniques we are using so be super super cautious and go slow and yeah, communicate in a precise way, adding demos, um maybe when adding a good patch, um try to drop in a little link on how to work with those, because this can be overwhelming, but other than that.

B

How do you feel about it? Dave.

A

um So I'm going to give a miniature shout out to both amelia and sarah from the monitor stage, with sarah's former monitor my p.m. When I worked there, um they.

B

A

Awful lot about properly interacting with pm and ux on merge requests and the big things that I think were really fantastic as part of the merge review was getting pm and ux in earlier, rather than later, to reduce wasted feedback time and make sure you're going in the right direction. This.

B

Really holds especially.

A

True for ux, you want to make sure.

B

A

You're doing is actually the polished product that's going to go into the production code base with pms, offering a very succinct how to test guide, because not everybody is super technical, as you've mentioned. So it's really on me as an author to offer up an easy way to get in and adjust the changes that we've made and to make the code review process much much smoother.

A

But I would also, on the other hand, encourage both ux and pms to lean in to getting involved in the code review process. It doesn't have to be just engineering absolutely.

B

Let's see if there are any more questions in the chat.

A

No, I think that might be everybody is us.

B

No! No, yes! Thank you. Austin yeah.

A

Thank you, everybody for attending. It was a fantastic session. Hopefully you come away with this with some tools in your two belts for better code reviews and me and yannick's next goal is to actually get that code merged to production so wish as well.

B

I'm gonna do our very best yeah from my slide as well. Thanks everybody for taking the time and have a great country see.

A

A