We can't find the internet
Attempting to reconnect
Something went wrong!
Attempting to reconnect
Analysis Summary
Worth Noting
Positive elements
- This video provides a highly relatable and practical critique of the 'LGTM' (Looks Good To Me) culture in software development, offering actionable advice on how to make code changes more digestible.
Be Aware
Cautionary elements
- The use of 'revelation framing' (claiming to uncover 'secrets' that 'no one ever speaks about') is a rhetorical device to increase the speaker's perceived authority on a common industry problem.
Influence Dimensions
How are these scored?About this analysis
Knowing about these techniques makes them visible, not powerless. The ones that work best on you are the ones that match beliefs you already hold.
This analysis is a tool for your own thinking — what you do with it is up to you.
Related content covering similar topics.
Don’t worry, I made sure to ask my LLM to do a security check on the code base before prod 🤓
Cognitive Class
What Could Go Wrong?
Groxio
Github might be in trouble
The PrimeTime
Transcript
Give a warm round of applause for Sasha. Sing through me. Oh muse of a pull request so fine which presents the change with clarity divine where commits weave a tail both true and fair so that no reviewer shall ever despair. [Applause] This is a tale as old as time. It is a tale of empathy, consideration, caring, and also of sharing, communication, collaboration. It is a tale of a pull request. But what is a pull request anyway? It can be many things. It can be the last step, the final hurdle we have to overcome to bring our changes into the main branch. It can be a request for feedback, cry for help. It can be a place where we evaluate, discuss, experiment with different ideas and approaches. It can be a historical record of a change. And then perhaps it can be a stage, a special kind of a stage designed for a special kind of a dance, a dance we call code review. Now this dance, it can sometimes be magical, magnificent, glorious. It can fill our hearts with joy and laughter. It can bring enlightenment and clarity to the team. It can make our code base a better place. And this usually happens only in a fairy tale. The reality ls is not quite so pretty. Far too often participating or doing a code review is a very gut-wrenching, soul draining experience. It leaves us tired and exhausted from all the endless discussions, pointless debates, nitpicks, clashes of the egos. And if we're being honest with ourselves, this code review thing, it's not the most favorite thing that we do as a part of our regular job, right? It's not like when we are asked to review a pull request, we all go like, "Yay, another PR. I'm going to review some code. This is going to be so much fun." Not not really, right? It's probably something along the lines of like another one again. Like does it ever end? I'm not even thinking about this right now. Maybe tomorrow if I'm in a good mood. And there are reasons for this. Many reasons I will not go into but I can recommend an excellent resource on the subject, a great book which is aptly titled Looks Good to Me by Adrien Bansza. This is really a detailed treatment of code review filled with a lot of practical advice which will help you improve this practice, make it a more pleasant experience and reap more benefits out of it. So give it a read. Highly recommend it. But in this talk I want to focus on one thing and one thing alone and this is a pull request. And I want to submit here in front of you, my dear audience, that the way we build, create, organize our pull request, the way we design and build this stage in many ways decides the look, the feel, the shape, the appeal, and the fate of the dance taking place on it. Here's why. You see, when we assign a PR for a review, what we essentially do is we share information. And the other term for this is communication. Now there are three key pieces of information we're sharing in a freshly assigned PR. So there is a title, there is a description and there are the changes to the code themselves. And in my small experience, you know, I've been doing a little bit of code reviewing. So for the past five years, it's number one activity I've been doing. I'm really reviewing more than doing anything else, more than development itself, you know. So I could literally put code reviewer as my job title on LinkedIn bio, you know. I didn't do it because nobody would hire me for that. That's the only reason but that's the reality of it right and so in that small experience working with some teams and different people I would say that we typically do a solid job with the first two bits of information so the title and the description and then predominantly we completely fail to organize the changes to the code and there are a couple of different patterns that I see I'm just going to mention a few of them not all of them so the most obvious one is when all the changes are simply handed through a single commit like there you go make some sense of it you know good luck Um another pattern which I see quite frequently and it has been mentioned around in conversations here is like we see a history there are some commits you know they seem to be small but when we take a look at this the titles we see something along the lines of work in progress or wip full bar buzz blah more wip make it work make it really work make it really really work this time it's definitely going to work now this kind of history it makes no sense not even to the author, let alone to the reviewers. And so we are forced to look at the total leaf of the changes and then basically it's just the same as the thing number one, except it has this added bonus of useless garbage of history. Another thing I often see is the situation where the history seems to make some sense, but if we take a closer look, we're going to see something along the lines of update dependencies, add a new dependency, implement the entire feature, fix the CI build, add a change log entry. And so there's like this one huge commit which contains like 99% of all of the changes. And so it's just more of the same. And that's a bit unfortunate because in many PRs, I would say in most PRs, the vast majority of the information resides in the changes to the code themselves, right? Many pull requests typically change or modify hundreds of lines of code. It's not uncommon to see thousands of lines of code being changed by a single PR. That's quite a lot of information. And there are different type of changes taking place there. So we have stuff such as changes to the database structure aka migrations, modifications of the ectos schemas, modifications of the business logic, perhaps something has to be refactored to accommodate for the new view of the world. Presentation aka interface logic, uh tests, documentation, and who knows what else. And all this stuff, it is crammed into the single committee into the single huge diff. And to make matters worse, it's not just simply concatenated. No, no, no, no, no, no. This stuff is haphazardly thrown together to form this entangled bash of a spaghetti which is overcooked and served without the sauce. Of course, the taste is terrible. Of course, it's hard to digest trying to make some sense of it. It's a frustrating experience. It is mentally taxing. It is energy draining. It is time consuming. And in most cases, I would say from personal experience that for average people such as myself, it is downright impossible to properly understand these changes. But still we try, right? What else can we do? We are asked for a review and so a review we shall provide. But how can that which is unreable be reviewed? Now there are two tricks of the trade and no one ever speaks about them. We have to come up with them on our own and we do because we have to survive, right? It's a self-preservation instinct. But today in this beautiful Senic city just in front of you, my most cherished audience, I will publicly uncover those secrets. Okay, are you ready? Secret number one, just approve the damn thing, right? So, I have no idea what goes on here, but like I'm going to trust my colleague that they done a good job, you know? So, uh, and even if there are some problems with it, they're going to take the fall, right? Not me. Because you see, g blame, it always points to the author, never to the reviewer. Okay? So yeah, I'm not going to provide any feedback. Instead, I'm going to do add some like nice uplifting message like nice work, good job, LGTM, throw in a cute emoji for good measure, like a heart, a thumb up, or a rocket. Hit the approve button and I'm good to go. Right? Takes a few seconds of my time and I can go back to my own job of producing my own monstrosity of a PR. But there is a big limitation with this technique. And this is the thing that you cannot really use it all that often, you know, because it's going to look suspicious. People are going to be on to you. They're going to figure out that you're faking it. Because you see these teammates of yours, they weren't exactly born yesterday, right? So you should never underestimate them. You should always assume that they're at least as smart as you are. So you have to do most of the time, you have to be a little bit more sophisticated, right? You have to build and maintain the illusion, your street cred that you're a thorough, tough reviewer. And the way you're going to do this is you're actually going to provide some feedback. But because you know we don't really understand what goes on here, what we're going to do is we're going to focus on generic, you know, superficial shallow stuff. So like perhaps this function seems to be too large. You should split it. I have no idea what it does, you know, but you know, split it in a couple of smaller functions. Maybe I, J, and K are not the best of variable names. Again, I have no idea what this code is doing, but doesn't matter, right? Maybe the documentation is sparse. Maybe uh type specs are missing. We can always nitpick about um about uh layouts, line breaks, uh grammar and spelling, British versus American spelling. This is a gold mine for making a lot of feedback that and Oxford comma, you're welcome. Right. So I don't want to say this stuff is useless, but it feels to me as if we are trying to inspect a building completely ignoring the foundation, the architecture, the layout, the design, the structure, the utility, the usefulness of it all. And we focus on the color of the paint on the walls, making comments along the lines of like, you know, I think this should be turquoise instead of cyan. Like, do we actually make some significant contribution here? If we're being completely honest with ourselves, too often the answer is no. And so, as a result of that, and as a result of us basically approving stuff we don't really understand, more serious structural issues uh can more easily find their way into the main branch and into production, right? to things such as incorrectly understood requirements, unhandled edge cases, insufficient tests, uh crashes, memory leaks, privacy issues, security issues, all this stuff now becomes the responsibility of the entire team and it can hit the production users. So that's a little bit unfortunate, wouldn't you say? But I mean, in our defense as reviewers, we can say like, look, the PR was as it was. I did my best job to make some sense of it. So, you know, I can only work with what I've got, right? Despite how we like to call ourselves, we elixir developers, we are not actually alchemists, okay? We cannot make a delicious soup if the only ingredient we have is poop, unless it perhaps comes from a bird, but that's a different talk, you know. So, it is what it is, right? Or is it? Perhaps there is another way. A road less traveled by which if taken might make all the difference. And it goes something like this. So when we are asked to review a PR, what we're going to do is we're going to take a deep concentrated serious effort to actually make some sense of it. Right? So we are actually going to try to be professional, not just look like ones. Okay? But we're not going to take a lot of times up to 10 minutes at the most. And if within this time we see that we are making little to no progress, we give up. We inform our author that we were unable to understand the changes and as a result of that we cannot really provide an approval but we also cannot provide some feedback which would lead an to an approval right that's the reality of the situation and we should call it like it is right we are essentially returning the PR on the ground of it being unreable right we should normalize this stuff like in my career which now spans decades I have seen this done exactly zero times you know so but that by itself is not going to be enough of course we should offer help to the author, right? So, bounce some ideas with them. How can we focus this PR? How can we make it more understandable and reviewable, maybe pair with them if we can, right? We want them to get some meaningful feedback and ultimately merge their changes, right? They're the part of our team. Their success is our success, right? But faking a review is not really helping anyone. More importantly, we want to spread the culture across the team of making such PRs by default which are nice and reviewable. But what makes a pull request reviewable? So there are two simple things, two properties, and this is honestly such a lowhanging fruit standing there right in front of us waiting to be taken that I'm absolutely amazed, astonished how rarely do I see people reaching for it in practice. So thing number one, slash the scope. A pull request does not have to implement the entire feature from start to finish. A feature can and often should be spread across multiple pull requests which are smaller in size and scope and hence they are much easier to review and properly understand. So what a small means in terms of lines of code which is admittedly a somewhat of a shallow metric but still uh here's my guideline. The vast majority of PRs should exceed should not exceed 300 lines of code modified up to five probably fine above that it becomes exponentially worse. Now there are going to be exceptions of course. So like the simplest one I can think of is maybe you're renaming some widely used uh domain term. So full becomes bar and so you're going to do like search and replace touch hundreds of file thousands of lines of code and it's fine to be to make it a single PR. You don't want to really break this thing. It can be even a single commit. you know it's easy to understand and this is really what you're optimizing you know how easy it is to understand these changes properly right for an average regular member of a team right so someone who is reasonably familiar familiar with the technology and reasonably familiar with the project so I'm not talking about juniors people new to the team I'm not talking about gurus such as Jose Valim you know regular person such as myself and for us it should take I would say typically no more than 10 minutes to properly understand all the changes Again, there are going to be some exceptions. Maybe some fancy algorithm is implemented. So, it's a head scratcher and takes a bit more time. But those are like rare exceptions. Now, this brings some important benefits for the entire team. So, first and foremost, obviously, you're the feedback loop is shorter, right? So, you're going to receive your feedback much sooner. Obviously, because it takes much less time to review the PR, but also because people are going to be inclined to review sooner rather than later, right? So, like I have 10 minutes of time on my hands. I have just pushed something and I'm waiting for the CI build to finish. Like, am I going to start a review or not? It depends. If typically I expect these monstrosities of a pull request, then the answer is no. I'm going to take a coffee break instead. And maybe I will start reviewing your stuff at the end of the day, maybe tomorrow. Sorry, not sorry. Now, if the pull requests are typically nice and short and understandable, then sure, I'm going to grab one. Maybe if I'm lucky, I can squeeze in two reviews in this small amount of time I have, right? I want to get this stuff off my plate and I want to help you. I want to unblock you as well, right? So, you're going to receive your feedback really much sooner. And this feedback is less likely to contain a bunch of this useless spam and nitpicks, you know, just because people are trying to fake your review. So, that might mean less useless work for you, the author. And the kind of feedback that you do receive is more likely to identify serious structural big issues, the kind of stuff you want to have identified before you merge your changes. Right? So in summary, all of us, the author and the reviewers are faster and we are producing the work of better quality. Like what are we talking about here people? Make your pull request smaller. This is my tip number one. But that by itself is not going to be enough because even when small and I speak from experience, it can be difficult to understand. So in addition, what you have to do is you have to present your change incrementally gradually. And for this you use commits. So think of it like that. If the entire history of your project was turned into a book, then features are chapters of that book. A pull request is a is a section in the book, short section, one or two pages long. And commits are paragraphs. So each commit adds something to the table, a little bit of information, another one building on top of it, more information and more and more and more. Commits are the fabric of your pull request. Use that fabric to weave the story of your change. because this is the story you're trying to tell to your reviewers. Okay. So, let me make it a little bit more concrete. Let's make a pull request. And to do this, we need to change make a change in some project. And as it just so happens, I have a project on my machine which is in a dire need of a change. And this project, it is called Hamlet. And the reason why it bears this name is because like many other projects, it is currently steadily marching towards a tragic fate. But you see, we are the authors of the story here, not some random blog from the 16th century. And so we get to decide the fate of our hero. And we're going to do everything in our power the best we can to ensure that our hamlet lives long and prosper. And the way we're going to do this is we're going to collect some metrics. So we're going to watch over the runtime behavior of our system and we can see if things are taking the wrong turn and we can take some corrective measures perform a divine intervention if you will which will prevent our system from making some unfortunate decisions with tragic consequences. And so specifically we are going to measure average utilization of beamulers. Now as many of you know a beamuler is a OS thread which runs our code. It is a processor of our instructions. And for this story it is relevant that there are two types of schedulers. there is a normal and a dirty or a dirty CPU one. Uh their purpose doesn't really matter, but it matters that we have these two types. And it also matters that there are multiple instances or threads for each type. So by default, you're going to have as many of each as there are available CPU cores on the host machine. So like on this modest machine, I have 20 CPU cores. So I'm going to have 20 normal scheduulers, 20 dirty schedulers. And I want to calculate the average utilization of all of them, which basically indicates how loaded or how busy my system is, right? And so I can see like I can plot this on a nice dashboard and I can see like am I reaching the maximum capacity of my my computational capacity on my machine. I could maybe have some alerting logic in place which informs me about that. Uh or I could have autoscaling which is based on this metric which then adds new machines to the cluster if needed. Right? Uh obviously of course we're not going to implement all of that. That's just not enough time. You know I'm not actually going to finish the pull request. I'm just going to start and make a couple of commits to illustrate some points. And then in the third act, I'm going to show you an already finished pull request which is available uh in the repo on GitHub uh in the repo which is called Hamlet under my uh GitHub account, right? So you can take a look at it and study it at your own convenience. So let's measure this average utilization. So what what you're going to do is you are going to like do some research, right? These days probably you're going to ask AI a little bit about it. Maybe on forum, maybe you can read the documentation. And I don't know if anybody anyone does this these days, you know, but that's another option. And either way, like if you're lucky, so can is the point okay? Can you see it? Yeah. So anyway, like if you're lucky, if you're done your work diligently, uh you're going to stumble upon the function which is called sample, right? And so this function basically takes for each scheduleuler a sample of how active it was since the dawn of time until now, you know? So how long of a time in some abstract unit was it actually doing something? So we get this for eachuler. So we're going to get the sample and I'm going to get another one after a while. And then I need to like calculate the the difference and utilization. For this there I have a convenient function called scheduleul utilization. I pass the both samples. And then what I get is the breakdown for eachuler. So the first element is a tag. CPU means dirty CPUuler. This is the ID. We don't care about that because we're going to aggregate all of that. And this is what we care about. This is the utilization. uh zero obviously meaning 0% and 1.0 O meaning 100% and then the formatted value as erlank string or char list and so we get this for each dirtyuler and for each normal and here we have like some values and we want to calculate the average of all of these values but luckily this stuff is already computed for us here u in this tupole which is like triplet here so total so I have the average of all of the scheduulers and so I can use that thing so okay now I know like what I have to invoke and uh the idea unfolds in my mind So what I'm going to do is I'm going to periodically uh every x I'm going to get the sample compute the utilization store the sample and utilization memory expose the utilization via some getter after a while rinse and repeat okay and if you're reasonably familiar with OTP then this is basically gen server it's a really great fit for this type of problem what you want to do is build a gen server which ticks periodically that's how I call it so you basically use process send after to ensure that handle info is invoke every x like every 1 second for example And you do the stuff that I have just explained. Store whatever you have to store in gen server state and then expose it via gen server call via some getter. Right? So we have the idea of the solution. And so now I can actually start coding it. Right. So uh I need to create a branch uh as soon as I find my mouse. Oh there it is. Okay. Oh yeah. But before that there is uh one personal confession that I have to make. Okay. So just between you and me what happens in Chattanooga stays in Chattanooga. Okay. I use git from a guey, okay? And I don't even know how to use it properly from a command line. Not even the basic of stuff, right? And I know that this is shocking, right? It's inconceivable really. You see, for years, I have roamed the pits of the internet, places deep and dark and terrible such as hacker news, Reddit, and ex Twitter. And in those journeys, I have learned that real programmers only ever use git from a command line. And so I suppose it means that I'm an unreal programmer. And you know what? I'll take it. There are worse things in life, right? So, uh, sorry for that personal aside. Uh, let me move on to my GUI of choice, which is called SmartGit. And so, uh, yeah, no affiliations whatsoever. I just, I'm just a happy user for like more than 10 years. And I'm going to open up a branch and let's call it measureuler utilization. Okay. So, now it's time to actually write some code. But maybe before banging on the keyboard, you know what what's worth doing is thinking about like how am I going to actually implement it. So I know the idea of the solution, but I cannot write everything all at once. So I want to work in some small steps, right? So I could, for example, first implement a gen server which ticks every second does nothing else just set up the ticking stuff and then I can expand on it. I can add sampling logic. I can calculate average utilization. I can expose this thing. I can start this thing in a supervision tree. And then probably it's time to actually write some tests which will require some mocking because I rely on things that I cannot control and I'm not really sure how that's going to look like. So I'm going to cross the bridge when I come to it, you know, but I have like a reasonable plan that takes me uh quite along the way, right? So okay, let us write a gen server which uh ticks regularly every second. So I'm going to create a file here uh and let's call it utilization.ex. And so it's time to write the gen server which ticks for example every second. But you see another short personal confession. I'm not going to lie. I'm a little bit anxious here. You know standing here in front of you. I have a reputation to protect and I don't want to make some rookie mistake. Uh so I am going to invoke some assistance. You know I'm going to invoke a magical mystical powerful creature. A creature not made of flesh. An unreal programmer itself of a sort. And this creature it is known to us mere mortals under the name of set Claude set 3.5. So let us ask Claude to write us a gen server which ticks regularly every second. You see can you I cannot enlarge that font unfortunately. So let's see what happens. [Applause] Okay. So um okay so it generated something. Let's see if it works. Uh so what I'm going to do is I'm going to open up a file in the temp folder. Test exs like a script. It's temp is by default get ignored. So uh it's not going to be accidentally committed. And so what I need to do is uh it's called Hamlet scheduleuler utilization dot uh start link. Okay, something like this. And let's see if it works. So it's mix run minus minus no hold temp slash test exs. And let's see. And is it ticking? Oh, we didn't add any sort of uh outputs. Usually it does that. So, I'm going to do this here. Yeah. Okay. Beautiful. Thank you. So, let's see now. Oh, that's in in its wrong place. My bad. Uh, so, well, there is actually handle info here. So, it schedules. Actually, it doesn't work. I wonder why it's not working. Oh, I know exactly why it's not working. I actually have to pass an option. Oops. Sorry. So like this. Let's see now. Yeah, finally. Okay, it's working. So, so yeah, you know, I have to confess. I was a bit skeptical at first, but now I'm totally sold. The wipe coding hype, it is real people, you know, like even a 16th century wannabe poet can write concurrent code these days, you know, like what a time to be alive, right? So, uh anyway, let's take a look uh what what we got here. So, thank you, Mighty Claudius. Uh, praise be thy name. Hamlet she'll never forget. So, let me sprinkle just a little bit of fairy dust on top of it before moving on. Right. So, let's see what we have. We have gen server start link. So, this thing takes args. I like to call it arg uh before I know actually what it is. We need its thing because of how we use gen server generates or injects child spec. This is all explained um in the documentation and also in the good book aka elixir in action. By the way, I have one copy to give away. So if you want it, you didn't read it, just to reach out after the talk. Uh, so I'm going to remove this name. Try to remember this moment. It's a very important moment for something that happens much later in the story. Um, and so let's say further. So we have this message is called Royal Heartbeat. Maybe let's not be so poetic or dramatic. So let's just call it tick. And then let's be consistent and let's just use tick as well here. Like that. And then we come to the a pet peeve of mine. So let's use timer seconds one. Um and the final thing that I want to do here is uh I want to do at impul gen server. So please don't be lazy and do at impul true specify the callback module explicitly. Let's see if it's still working. Okay. Yeah, it's still ticking. And so now I can also drop uh I can also drop this IO puts. And okay, now I can start uh writing the sampling logic, right? Uh but before I do that, I'm actually going to commit these changes. So I'm going to move on to the tool. Let me just quickly double check the diff. Yeah, it seems fine. And so what have we done here? We are um we are uh we have created periodical ticker, right? Nice and short name describes what we've done. Short diff and so that's good. Okay. So here comes the big moment of this talk. Prepare to be blown away. Are you ready? >> Yeah. Okay, work in small steps, commit each small step. It's not exactly groundbreaking stuff, is it? Right. Uh and yet I see it done so rarely in practice. And one theory why that might be the case is because we fail to preparate to separate the preparation from the execution. So we kind of try to code our way through the entire problem without actually doing some initial exploratory work. And so we are stumbling in the dark and our history basically reflects that thing. You know this is why you end up with full bar buzz blah and similar garbage of comets you know so do a little bit of upfront analysis like understand the problem figure out which functions you have to invoke to actually solve that problem think about how that works in the larger design of your existing codebase and now you have the vision of you know uh the solution and then you think about which small steps you need to do to actually take you there and all of a sudden you have this beautiful yellow brick road which takes you from start to finish and gives you nice rhythm nice pace. So with each step you are dealing just with one thing, not with everything all at once and not with all that uncertainty. And so you will actually probably be faster too, right? Because working in small steps, it's such a universal advice that applies for every activity in life, you know? And your reviewers, they will feel as if someone transported them from the stone age into the wbe era. They will be so much faster. They will see things so much more clearly. They will not know what hit them. Right? So again, everyone on the team is going to be much faster and this is possible because we have done a little bit of upfront preparation and planning. So as the old saying goes, weeks of coding can save you hours of planning. Never forget that one. Okay, so do a little bit of preparation. Okay, so let's move on. We need to implement the sampling logic. And uh for the rest of this talk, I'm going to do something barbaric. I'm going to manually write some code. Okay, forgive me for that. So what we need to do is uh we need to let me do this in I need to do this in two places. So I need to take a sample and it's going to beuler sample. So I get this thing and then I have to store it in this into the state. I actually had this autocomp completion turned off but apparently some updates change this thing. So because I don't really use this auto completion but okay anyway. So I put this thing put the sample here and that's going to be enough for the moment. Right. So let's take a look. uh at how this works. And to do that, I need to actually take the pit of this thing. So, it's okay pit. And then I'm going to use cis get states to pass this IO inspect. I don't think I need this label. Uh uh and what I'm going to do then, I'm going to do this again after a while. So, not 20,000. Yes, thank you. So to ensure that one tick has happened. So let's see what what this gives us and it crashes function sample is undefined because module is not available. So for this you need to add an extra application. Uh and this application is called runtime tools. Yes, thank you. So uh it's a part of stalker lang distribution. You typically want to add this stuff if you want to use observer with production. It's going to simplify this work for you. The work of you know troubleshooting production. So something is now printed and after two seconds something else is printed. I don't know if these numbers make sense. So we're going to take a look at this a little bit later on. Right? But so okay so now I have this thing. Let me just check the diff. Uh let's just maximize this. Okay it looks fine. And here I have added the extra applications. So I'm going to commit this thing. And what are we doing here? I am uh for example collecting samples. Right. So you might be uh tempted to split this into two commits. Don't do that. The diff is small. Uh it's easy actually easier to understand when it's together. So like you're going to see that I have added uh this extra application and you immediately see what I'm using in there in the same diff which is on the same page on your browser. And most importantly, we can give it a logical name. This is the name of our step, right? Uh speaking of names of your commit messages, if you're struggling to produce short titles, if you feel you need to use AI to summarize your changes, then uh changes, chances are very high that uh your commit is unfocused and you're doing way too many things. Okay, it should take you no more than a few seconds to give like this kind of a short and nice name. Usually you don't have to write novels in the rest of the message if the diff is uh reasonably short. Okay, because it speaks for itself. So, okay, let's move on. And we are collecting samples and now I have to get the average utilization. And so what I'm going to do is something along the line of average utilization. Uh no, this is why I don't like it. Please don't hallucinate. So average utilization equals average utilization sample and state sample like this. Um and what I'm going to do then is I'm going to store this thing into the state. And to be able to do this, I need to have it in the sta. Yes, initially it's 0.0 uh because I don't have two samples. And so now all I need to do is implement average utilization. Um what is this thing given? Uh this is definitely not what I want. Uh so utilization takes sample let's make it sample one sample two because this is actually incorrect what it writes. Uh and once I have this utilization what I need to do is enum find uh the the total triplet in that utilization right. Uh so something along the lines of um I have so what do I have here you know find utilization uh I have no not like this I'm going to use and match so it has to match total value formatted which we don't care about and actually I don't care about this thing here either uh so this has to match this think okay and then I get total value formatted and I return a value okay something like that let's see if it actually works uh yes thank you okay so I get zero and then I get something which is just barely above zero does it make sense again we're going to take a look at it a little bit later but I'm computing average utilization and so let us commit this thing uh calculate average utilization right and so finally what I can do is I can expose this thing so let me just do uh average it takes a pit and does gener call so average from state and this is this is actually good so let's see what happens here. Oh, and then I I should obviously use this function. So now I can finally replace this get state and use hamlet scheduleuler utilization dot average, right? Okay. So it's zero and then it's something above zero. So this is unfolding nice and smooth. So we are exposing average utilization. Okay. And so now maybe it's finally time to actually see if this is working well. And to do that I'm going to spawn a function. You're too eager. Okay. So what I'm going to do is I'm going to uh put the singleuler to 100% with the CPU bound loop. And I can do this in a bunch of different ways. So for example stream cycle and then pipe this thing pipe this thing to stream run like this. Okay. Uh and furthermore because I have like 40ulers here I'm going to do I'm going to remove the number of scheduulers. So it's system info um schedulers online I believe. And I'm going to set this thing to one. And this basically means I'm still going to have two times 20 schedulers but only one of each is going to be used. Okay, so I have twouler threads. Uh one of them normal one is hammered to 100% another one is uh 0% and so I expect 0.5 or 50%. Okay, so let's see if that actually happens. Let's give it a try. So 0.5 is what we expect. Uh it's not system info, it is a system flag. That's my bad. Not f flag. Okay, let's see now. So zero and then 0.025. That's considerably smaller. Perhaps it was a glitch in the matrix. Let's try it again. So no, it's wrong. Consistently wrong. And so it means that something is rotten in the state of my gen server. And so let's see what that something could be. And uh yeah, this is the problem, right? So this total triplet um it actually calculates the average of all of those uh scheduulers irrespective of whether some of them are offline or not. Right? And so what I need to do like this algorithm actually isn't good. I need to uh calculate the average myself. And so what I'm going to do here is something along the lines of so let's see four. Oh man, can you stop doing this completion? So for uh I get normal and ID I don't care about and then I have value which I care about and then I have formatted in the utilization list I'm going to return a value right and this gives me normal utilizations like this okay and so what I'm going to do then is I'm going to do the same thing for dirty utilizations except Here it's CPU. Uh and then I'm going to enum take from the normal utilizations as many as there are Erlang system infoulers online. And then I'm going to concatenate this. Whoops. See, I actually want to take this from dirty utilizations as there are dirty CPU schedulers online. And what this gives me is the list of online utilizations, right? And so now all I have to do is calculate the average. And this is basically enum sum of online utilizations divided by length of online utilizations. You might worry that this is potential division by zero. But this list is never empty because if this code is running, it means that at least one scheduleuler is online. So QED. Okay. And so uh yeah, let's first see if it works. So zero and then yay 0.5. It works in the first attempt. You know how often does it happen, right? is Christmas already and so uh yeah the one thing that I should do here is I should explain why am I doing this uh why am I not taking that total thing uh uh from the list obvious I I hope at least that how is apparent you know given that I have been using some paragraphs and some descriptive naming but the reason is maybe not uh apparent as much right and so now I can commit this thing but here is the thing you know so like uh when the reviewers see the history they're going to see my first failed attempts like do they actually have to see it. Is it some useful information for them? The answer is no in this case because I completely overwrite the algorithm. And so I will actually try to bring this change uh two commits earlier into the original thing. So what I'm going to do here is I'm just going to select the same message calculate average utilization commit and then I'm going to rebase from that thing and I'm going to just auto squash the first commit and rebase and let's see and it works right. So when I see the log, what am I doing here? I create periodical ticker collect scheduleuler samples and then calculate average utilization. And in this particular commit, you immediately see the second version. So you are not aware of my failed attempt. And I don't I'm not doing this to make myself look perfect, mind you. I'm doing this to reduce the amount of uh useless information for you to make your job easier. And this is actually part of my standard flow. So like this is perhaps a more complex example, but often I for example forget to add like a type specification or maybe I figure out like three commits down the line that like this name was crap. you know, I should have used a better name. And I'm going to try to bring this change to the original place. Okay? But if I get a conflict, I'm going to abort, right? So, I'm not going to like take a lot of time to make this history like super perfect. It's fine if it has some bit of imperfections, you know? But if I can help it, uh, I'm going to try to clean it up as much as possible with little effort. And this is the whole idea, you know, we should do these things not because they're hard, but because they're easy and because they improve the quality of life of the citizens of our team, right? So yeah, there's quite a lot of stuff that I have to do here, but obviously I'm running out of time. So uh instead, without further ado, my most beloved audience, I present to you the pull request. Feast your eyes. Marvel at his glory. Behold the commits. They tell a story. Right? So like even without actually reading the description, you know, if you take a look at the titles, what are we doing here? We are creating periodical ticker collecting samples computing average utilization exposing it run this stuff in the supervision tree and then the rest is all about testing and mocking right and so you can see how the story unfolds and you can literally tell the scope of the story like I'm not dealing with metrics I'm not dealing with sorry with the dashboard with alerting with autoscaling here that stuff is left for another day this is an example of partially implemented feature but it's like nice self-contained we are talking about 150 lines of code you know spread across 13 commits So on average that's I think less than 12 per each uh commit. Obviously some are going to be larger, some are going to be smaller. Uh let's take a look just at a couple of them again. So this is the first one. You know, nicely fits on the single page. It's like actually larger than most of them, but nothing fancy is happening here assuming you're reasonably familiar with OTP. So you're going to spend like maybe 20 seconds, 30 seconds reading this thing, and then you can move on to the next thing. So this is where I'm collecting samples. Just a small note that this is slightly different than what I have coded in front of you. Too long didn't read. This is more production ready. The stuff I was using was more demo friendly. Uh but roughly speaking, it's the same thing. U and then we have the algorithm here which is perhaps the most complex thing in this whole uh pull request. Maybe that and uh mocking, you know, but here's the nice thing like you read this thing in isolation as a reviewer. So here you are focusing on this average calculation and nothing else. And once you're past this commit, you don't care about how it's actually implemented. You have reviewed this stuff. Okay? But this makes your job much much easier, right? And so, uh, one final thing I want to show you or one final commit I want to show you here is this one. The first one I didn't live or wibe code. Uh, so this is where I'm starting the thing in the supervision tree. And notice how I'm giving it a name here, right? So I mentioned earlier like uh when Claude generated the name, I actually dropped this thing. So this is where I need the name because I'm running this thing in a supervision tree and I'm interacting with this thing. So it's a singleton process in the in my supervision tree. So the typical solution is to give it a name, right? This is where I need it and so you can see it, right? But uh so here's the thing like there is this technique in storytelling uh which is called the setup and the payoff right so basically in the setup you provide some piece of information usually somewhat casually and then much later down the line in the payoff you actually use this information to resolve some important part of the plot right so for example like you could start by saying that our protagonist knows how to pick locks and then the story unfolds we almost forget about this small detail and then by the end of the story or near the end of the story they escape the prison using their block picking ability, right? That's the payoff. So, this is precisely what you don't want to do in this kind of storytelling. Okay? We are not writing a piece of art here. So, forget about like red herrings, plot twists, suspenses, cliffhers, you know, uh nonlinear narrative and such, right? This story should be like the cheapest formula fiction, pulp fiction of the worst kind. You know, something along the lines of evil Claudius threw Hamlet into a dungeon. But luckily, Hamlet knows how to pick locks and the guards didn't take his tools from his pocket and so he escapes. Right? So, the whole idea is that the reviewers, the readers immediately see how these two things connect together, right? This reduces their mental burden. Okay? It makes things more simpler for them, right? So um yeah one thing that I want to say like every time I read such pull requests which unfortunately is uh too rare of an occurrence in my life you know it always looks the same like I just go through this commits like 20 30 seconds per commit you know next next and in few minutes I'm at the end you know I don't even notice that I'm at the end like I'm like why is it not going further like is my internet down do I need to reset the router uh and then I figure out oh I'm done and I understand everything it is possible the legends are true you know and you know What I do then LGTM cute emoji approve right so we've done the full circle but now it is earned it is earned by you the author and you will know when it is earned just like you will know when it is fake you know and when you know that like you have done a diligent job the PR is reasonably small and well organized when you receive your your approval after 10 minutes by a couple of colleagues it will boost your confidence right you will be confident that they have actually understood this stuff and that they think this stuff is solid right and this is perhaps the most important point of the whole review process and you can get this goal much more easily if you actually spend some time organizing your pull request. So yeah, time to wrap this up. Let me go back to my slides. Okay, so when all is said and done, when all the smoke lifts up and all the mirrors have been shattered, here's what remains of this talk. If you're practicing code review in your team, then as an author, you're not just trying to implement a feature, fix a bug, improve performance, reduce uh memory usage, uh refactor or whatnot. You're not just trying to resolve that Jira ticket. Okay? You also get to decide how easy or difficult it is for your reviewers to climb up to the top to reach that point where they fully understand your changes. You get to decide whether they can do it at all or not. And so kindly have some empathy, have some consideration, make this job possible for your reviewers and make it reasonably easy for your reviewers. You definitely don't have to go above and beyond to make yourself looks perfect because of course perfect is the enemy of done. But let's not forget that ASAP is the enemy of good enough. So try to find that nice sweet spot, that nice balance where just a little bit of your effort makes lives of many other people on the team much easier. And you're going to be faster, too, because what goes around comes around. And I can guarantee you that the quality of the pull requests you receive shall match the quality of the pull requests you give. And so with these parting words of sweet sorrow, my most esteemed colleagues, Alexorans, friends, this talk is done. The curtain falls. Gather outside. Discuss in the halls. May pull request a story tell. I humbly ask and bid farewell. [Applause] [Music]
Video description
gigcityelixir.com Beginner, hobbyist, or if you use Elixir in your day job, you will find talks here that change the way you look at code. GigCityElixir is brought to you by Grox.io #ElixirLang #GigCityElixir