 Let's talk about code reviews today. Welcome all for joining me also in the second room. I can see you But welcome for joining me. I heard that the second room is also packed. So officially next year I'm going to speak on the main stage You can tweet about this with PHP UK conference my Twitter handle and PHP UK 17 So my name is Hannes and I can describe myself in a couple of emojis or emoji. That's the official plural I can describe myself with the Belgian flag. I like to swim. I like to cycle I like to run and also love working with computers. I love making digital products The last one isn't really an emoji, but it's the company logo. I work for a company called made with love It's a Belgian company, but we're a remote company. So we also have people in France, Brazil, Canada basically all over the world Well, not in the eastern side of or not not to the east of Europe But mainly to the west we do PHP and JavaScript development for clients. It could be a government could be a startup could be anything But we're here to talk about code reviews, of course So what is a code review? Let's first outline the subject a bit A code review is someone who wants to make a code change But he doesn't Isn't allowed to make it straight to production So what he or she does is make a couple of changes and show it off to a peer Another programmer or someone else inside the company or outside the company doesn't matter Showing your code before it gets merged to other environments like staging or production that is a code review Who can do the code review someone else inside the company could be a consultant outside of the company Could be the CTO could be another junior dev could be anyone basically Not all code reviews have to be done by the CTO or the lead developer So what does it look like you push some changes to github or to bidback at whatever you want And someone else is going to review that He or she is going to look at the commits look at the individual lines that have changed and provide some feedback Right. It's a feedback loop based on that feedback The programmer will reply to some answers and make yeah reply to some questions with some answers Why did I do this? Why did we died? Why did I do that? And maybe initially also push some more commits to fix stuff and that's a circle So it's a basic feedback loop If you make the feedback loop fast enough, it feels like pair programming actually so In the end The reviewer will say oh well, this is finally in a stage state that I want to merge this into developer branch or master branch and push push it out to staging our production. So that is approving Not everyone has the luxury to work in a team with multiple people so you can also do a code review or your own code Before committing code you can do git add dash p To review individual chunks of changes before you actually commit them. So that's also pretty useful It's a one-man show or one woman show you can also work with git branches and GitHub and make your own Pull requests before merging them if you're if it's a one woman show or a one-man show Basically you review your own your own code in a different environment on a different screen than your IDE That's also useful. That helps with pointing out some failures or some flaws in your own code But why would we spend some time on that? Why wouldn't we just push to production and If it works on my machine works in production, right? well, basically before I Started doing this talk. I was at a meet-up And I was talking to some person and the person wasn't happy with his current workplace And basically everything he said why he hated working there was because they didn't do code reviews The code style sucked People are pushing things to production production would break More senior developers would revert changes every time When someone else had pushed something to master There was no communication between the developers So basically everything he said that he hated about the company was because they didn't do code reviews So it is pretty useful to spend some time on that. It's not a waste of time For the person that is submitting the code it is useful because They can learn from their peers. They get feedback on their on their code. Maybe there's a better way to do this maybe there's a different design pattern that better suits this situation as a Submitter of a code review you also start to feel more responsible for the product You first have to show your code what you did your changes to your peers It's Everyone coding is a craft So everyone likes to put out their best effort and show it off to their peers. Look what I did You start to feel more responsible for the product and for the code that you're putting out You're taking pride in the code that you craft It's also useful for the the peers that are reviewing the code. I Do maybe 15 hours a week on code reviews and I still learn every week PHP has so many functions. I still learn new functions every week. I still learn new things or creative ways of doing Of crafting a solution I can I can still learn from the most junior deaths in in my company and and in other companies that we work for It also it's also useful because I may not be working on some features that my peers are working on and I still want to know how that's how that is built I can still learn from that because maybe in two weeks time or six months time I will be working on the same features and I want to know up front how it's built. I Don't have to know but I like to know because coding is craft So it's useful as a reviewer too It's also useful for management So if you go back to your company and you go to your manager and you say I want to do code reviews Well, this is why it raises the level of both your juniors and your seniors They both learn from each other. It's making your team more coherent. They they start to work better together It also raises product awareness about about all the features. How do features work? How are they built? If if someone leaves the company in two weeks time That and the person has built a new feature three weeks ago and someone else is to take over But you didn't do code reviews. It's going to be a hell of a task to look into the code that no one has ever seen If you don't if you would have done code reviews, it's easy. Just jump in and you you've already seen this code So it it generates better code You get some feedback loop and you Improve the code before actually merging it. So better code means more maintainable code, which means Easier addition of new features That's what bosses like, right? some peach PCO like sweet capital stuff This is not really good. It's it's better to do less deploys and Have a feedback cycle before pushing to production. So What should you do when you're making a pull request? Right, it's not as easy as get commit to get push click create new pull request. It's a bit more First you get a task assigned right build a new feature remove a feature Adjust the feature read the task. What is expected? plan a meeting with with a product owner or Whoever made it made that task ask ask him some questions. What is expected? How should this work? Is this a filter with a With a slider is this just a plain text field? How will this look like? What is expected? What what do I need to make? Do I need to make a Gaussian curve filter or just plain text search? That's you know make do some analysis just do some research Before you actually start making the code Then When you start to code test first, right? Make a test what is expected an integration test? That's easy Make sure that the test fails and then start coding your way Well, that's the best practice, but I admit I don't always do TDD Right, so you made your test commit the test Make it go to see I see it fail Cool, it fails then add some more tests and some more commits Like add a new endpoint rename a variable make a new CLI command Test the backup Remove some conditionals Make some if else statements disappear by using a better way of coding do some solid design Remove something Do some code style fixing Make that a separate commit each time everything you do is a separate commit about commit messages I can go on and on about that for another hour, but After this it's lunch, right? So I won't do that Basically what I do is I Think this commit and then I continue writing what my commit messages this commit fixes the code style This commit renames this variable. I don't write this commit, but I Do write the rest so that's my that's my format of doing commit messages yours might be different, but that's okay So you made all the commits and then you push it to get up or bid bucket and Then you want to compare the changes to the the base branch develop maybe or development Then you hit create new pull request There's a big form here use that. It's useful Create a beautiful title. What does this do? right It this adds this new feature Some brief title and then use the big box below the description pull request description to fill out all the details You want to properly document this PR right a changelog, right? I removed this variable. I removed that feature. I added an endpoint. I Re-enabled backups if you're a git lab Link to the assigned issue or the task the person that is going to review your PR Will want to know why you did this. Why why did you add this endpoint? So the person that is reviewing your code can actually click that and go see what the assignment was and Then use that in the in the back of his or her head to review your code. Is this okay? Is this in line with the expectations? Then below the changelog also add some Description about the decisions you made why did you use this design pattern or why did you use this composite package? right So that it doesn't have to be rediscussed afterwards so that the the reviewer doesn't have to Add a lot of questions. Why did you use this or why did you rename this variable to? I instead of index Something like that then last step to the right assigned some reviewers one or more Right could be yourself Could be someone else could be multiple people Right, so that's your pull request great But I also Said something about code style and I want to give you a brief Intermezzo about code style We all know psr2 right you all know that Well, you may not know but soon. There's also psr12 with some peach peach seven syntax stuff And there's also symphony code cell which is what we use in our company This code style actually extends psr2 with some extra rules And on top of that we even add some more rules It's open source so you can look it up We add some more rules to make it easier to review some code I'm gonna give you some examples and then you say left or right which one which of the diffs Code diffs is easiest to review. Okay, first one. Which is the easiest? Left correct. Why maybe the green isn't so clear Yeah, smaller diff. So it just says I added one line The right the diff on the right is exactly the same, but there wasn't a trail in comma here So we added the multi-line array trailing comma rule to our rule set of code styles Because it makes it easier to review code If you look at the left one you see oh one line has been added if you take a look at the right one It says one line has been removed and then you add a two, but what does this functionally change? It's easier to see the left one that the functional changes in the left one compared to the right one And that is what you want as a reviewer. You want to see the functional changes You don't care about the code style you care about the functional changes That is what your task is to see what the functional changes are and to give feedback on that Next example, which one is the easiest? the right one right a Lot of people like to align stuff in their IDE so it's easier to read in columns But if you change one key of this Jason for example, then You might need to reorder in the entire column and that makes it harder to review Sometimes you have files with 300 entries in a Jason file And it's super hard to see what the actual difference is if you change one key and you make a super long key And then you have to reformat the entire file. It's super hard to spot what the single line that cost the change is so Forget about the alignment just Do this do PHP doc params disable it That would align the PHP docs dog block Also disabled the align double arrow Disable the align equals because that will if you assign stuff with an equal sign that would Align all the equal signs. I hate that. I hate that as a reviewer. I can't stand that It's hard to see what has actually changed Next one is braces Which one is easiest to read the right one, right? The left one didn't have changes Have a code block after the if statement So if it's a one line if block you can actually drop the curly braces But it's harder to spot what what has actually changed. Okay, this dark green bit helps a bit But it's not it's not enough It's super easy to see that. Oh, you added one subscriber here. That's it. It's super easy same for constructors you can new up a new item without using the braces Just use new with braces code cell rule if you use that you actually enforce The use of the braces and this one everyone likes this one, right? it's at the end of a file when you don't have a new line after the last empty line it says no GitHub says no new line at the end of a file and This week I saw this at a client They do slash slash slash for slashes. I don't know why and the file no new line It's it's crazy. Just use psr2 Add some more rules if you like and add that to your CIA CIA check So that everyone has their code cell enforced And it makes good reviewing code easier. So what should This is the end of the intermezzo. What should a reviewer do first we talked about what should the submitter do What should the reviewer do? Well first the don't just don't bash about code cell make it an automated tool make it a check in CI use a linting tool and force it It's not easy if you're working with a legacy code base, but Try to work towards a uniform code style Another thing you don't have to check as a as a reviewer is is this code mergeable GitHub will tell you if it's not mergeable. It will tell you if there are conflicts. So that's something you don't have to worry about Don't complain about duplicated code Two lines of duplicated code isn't bad if it's readable if it's clared it's more clear Then just leave it use some automated tools to report duplicated code It's okay. If there's two lines of duplicated code. Just use scrutinizer CI or something to notify to notify you about it, but don't start Commenting on code like make an abstract function for this It's not it's not necessary. If it's readable, it's okay. Just keep track of your technical depth use scrutinizer CI use Travis use CS fixed linting tools, whatever ES lint, whatever you want. Just use all those tools To make your job as a reviewer easier If CI says that something is not in on par with company standards or project standards, then the CI tools will tell you So basically everything that's automated. You shouldn't do that as a that's a don't One last thing that's not automated is Discussing the issue the person that has submitted to PR should already have discussed that as that person should already had the discussion and the questions and why is this What what should this look like? You don't have to rediscuss that Basically in an ideal world the the submitter of the code will already have done that If you see that the submitter of the code didn't ask enough questions Close the PR and say please rediscuss this because I I'm pretty sure that this is not what is expected so What you should do as a reviewer is Look at the PR. Look at the description the change log the design decisions Go to the linked issue as well to see what was expected and what is discussed If you read that first and then you look at the code you will easily spot that when something is not implemented the way it should and Then you can say maybe go back to the Project manager or the product owner to rediscuss the design of the feature After that you go look at the code and you look for things like the naming of variables the naming of methods Yeah, you look at design patterns. Are they implemented well? Do the correct classes implement the correct interfaces stuff like that. That's something that cannot be automated There's no AI system yet that can detect design patterns If someone ever invents that then we're all out of jobs Look at the clarity of the code is it readable when you have to Continue developing that feature in two or three months time. Will you be able to? Step in and start progressing Is it clear? Sometimes code is just too creative Sometimes one example We had to use slugs in a URL But the slug had to be unique. So when it's not unique We just add dash to dash three dash four until it's a unique and there was a while loop which Decremented an index and I said don't you want to increment like from one to two to three to four And then I noticed that there was no concatenation of the dash and Then after 15 minutes of tinkering. What does this code do? I found out that actually the the index decremented to minus two minus three and that was converted to a string and appended to the slug that's just Too creative. I had to think about that for 15 minutes to figure out how it was implemented It's crazy So I stepped in and I said It's not really clear. Maybe just make it super clear add a dash and then add the string version of the int Anyway So it needs to be clear clear ask some questions if something is not clear like Why did you name it like this and not use a Static factory method or something Always be asking questions even if You know just asking questions the answer to the question can be a valid reason for doing stuff like this could be The answer to the question could be oh, I don't know. I just implemented it like this, but maybe another way is better so Questions can still go both ways It's better than saying you should do it this way or You should do it that way because if you ask a question There might be a valid reason behind it behind the decision Also look at so we looked at naming and stuff. We should also look at Missing migrations. Will this deploy to production automatically? Will this cause any failures in Redis or something? Maybe we should clear cash when we deploy this stuff like that. That's something that AI cannot do yet Then when the cycle is complete and you approve this pull requests You should give some credits give a thumbs up well done At the ship emoji or a ship that's being shipped off Some some gif or something make it funny you make it Cheerful give some credits like I'm glad we finally can merge this Good job Also about the questions if you have some questions and you're discussing some some decision Maybe links from articles or blog posts could be useful Could be that this person or that person doesn't know this design pattern or or doesn't know a command bus Just link an article about command buses. It's a It's a way of educating the person without having to type an entire blog post in a pull request So the reviewer the Submitter has also been putting the effort in to make some atomic commits. So you can actually use this Go the previous commit and the next commit. I hate that this previous just jumps to the right OCD, you know If the reviewer has been putting in the effort to make atomic commits You can use this to look at how this person has been implementing this take it step by step It's like per programming but delayed if You're reviewing code and you've been Giving feedback on a person's pull request. You should also mind your language language is important I already said that you should be using some gifts or some emojis thumbs up ship It makes it more cheerful Also give some credit when when you when you learn something new if you see a function And you never heard about that function and super smart to use that function in this situation Just say so I learned something new. Thanks give compliments Use a gift like yeah looks good to me or If it's a super huge PR with 500 commits and you finally managed to ship it I feel sorry for the cameraman or a woman ouch So language language is important When you're reviewing some code Make it less personal Just step away from the person that has been putting out the code. You don't have to say oh my god I'm a big fan of your code fanboying is not really at It's not for this situation so another thing I like to do when a Pull request is ready to be shipped But you have some small amount of time left then I ask maybe Add this one more thing just go one step further give it the extra 1% just to see how far you can stretch the person How far you can make this person go to go to making the perfect code On the other hand if time is pressing and the code works there's some Issues with it some naming things, but basically it works Let it go. Just make it ship. It's also fine. Just depending on the time on the the time till the deadline Make your code reviews Change a bit if you have some extra time Push for better code if you don't have enough time let go of some principles and Ship the code Has anyone heard about the lizard brain? I see a few nodding heads. Okay The lizard brain is basically one of the oldest parts of our brain We inherited that from the lizards like alligators and stuff. I don't know much about lizards, but basically The lizard brain is a thing that Activates our offensive mode when we get attacked So when we get attacked we fight back It's an old part of our brain that does that and Basically, you can trigger that by using the word the wrong words if you say something like Do you even know this design pattern? Do you even know how to write JavaScript? That's offensive and people fight back So mind your language You're working with people. You don't want to offend them. They're they're your peers You will have to work with them maybe for a long time so better be friends with your peers and Use some suggestive language instead of saying you Make it about yourself right For example You see a pull request and you know that some things can be better Use some suggestive language like I would maybe Think about something You know Make it about you and say I would or I feel like If we if you figure out a way to do something like that Use maybe a lot or Or would or what do you think about this? Make some suggestions and then ask for their opinion instead of saying Instead of enforcing stuff So yeah, that's suggestive language Don't trigger their lizard brain The next thing this is a compound emoji. That's not a real one It's called the shit sandwich You know when you have to give feedback to a person Better wrap it up with something nice You might be giving some hard words or maybe touch their feelings, but better Start off soft right soft button on the bottom say like it looks like it works Good job. You've been putting in a lot of time and effort in this, but There's the next layer the shit layer and so funny I'll tell you this later. So go to the next layer and then give the feedback What do you think about? that class maybe change it to a factory or I don't know maybe we or There's already something in this project that can do that instead of rewriting stuff So maybe what do you think about using this class instead of writing your own? What do you think again use some suggestive language there and then after the feedback Return to the first soft layer. You want to soft bun to top it off with some sesame seeds or something say Something other suggestive like Maybe if you work with this or that person I'm pretty sure that we can collectively finish this off and ship this code and then move on to other things That's topping it off with a soft bun. So start off soft. Give the feedback You suggest with language there. You don't want to trigger a lizard brain and then top it off with something nice again right so that is How I like to give feedback and the funny thing is Once you know this technique you started notice when someone else uses it That's sometimes it's like wait, did you just But it's okay, it's okay, it's They know how to give good feedback it tells you something about them instead of telling us something about you did wrong and Then last but not least Running ahead of schedule here, but you'll be first in the lunch queue This box It's about merging. It's the last step of a pull request Who do you think should merge a pull request? The one that has been reviewing the code or the one that has been putting out the effort I see some nodding heads at both sides Basically what we feel is it's it's personal we work with a remote team and Some people might be working In the PM while we are already asleep in Europe basically whoever presses the button and triggers a deploy to staging has to be Has to be comfortable Fixing the deploy or fixing the incoming bucks neck errors because You don't want the staging to be down or a test environment for the client to test to be down When someone who actually can fix it is sleeping. You don't want to call other people in their sleep There's also This little link here command line instructions. That's also pretty useful Before merging it you might consider pulling in the code locally and test it out It's not only for QA engineers to test out stuff I you it's useful to merge this code locally and check it out before merging it into development so wrapping up here Code reviews are nice They're useful for your entire team your entire company. It's not a waste of time As a submitter I would like to urge you to review your own code before committing it or before sending in the pull request You might catch some stuff before someone else catches it Maybe a lost far dump or something Automate as much as possible put some more linting tools in CI some more testing environment testing tools smoke test whatever maybe a I just thought about this maybe some Fakes fakes on deploy. It's also useful you can do Capistrano deploy without actually doing it So reverting right right before you move the sim link Give some valuable feedback, but stay positive. Don't touch their lizard brain Try to use suggestive language and of course the shit sandwich if you see the shit shit sandwich being used against you Don't be offended. It's just well played Now I would like to all of you to go to your teams go to your bosses spread the world spread the word Give some good feedback and learn from your peers. That's it. Thank you There's a joined in link here. Please give me feedback. It's been a bit short But okay, you'll be the first in the lunch queue give some feedback to other speaker speakers as well Something a lot of people don't know is you can also give feedback to the conference organizers. That's also useful for them You can give feedback to them and come back next year to a better conference I would like to answer some questions now. There's a mic in this room and a mic in the other room. I Want to answer one question at least from each room. So As a code reviewer you can give quite factual Comments like this is how I would have done it or things like that which Sometimes being code reviewed people can get really offended they can take it personally when of course it's just about it's about the code But they kind of feel oh, you're saying that I'm a terrible developer How do you explain to them that it is just about the code that you do it to someone who was? 20 times more experienced and that you get this feedback and it's just factual and That you still think that they're a great person without them still feeling Yeah, but you said that I was wrong you can still make them feel valuable if you Append a little question. What do you think? make make it Make it a discussion about two equal developers Every developer is equally my in my eyes Yeah You suggestive language at the question and that removes 90 90% of the offensiveness of Feedback sentence. Does that answer your question or? mostly but in a case where I Think in a case where they do do something that's wrong, which is fine You can say like you can say that you've done some other great stuff But they still pick up that point that is objectively wrong And you can't necessarily make that a discussion. Is there a way of just like a really simple way of explaining to them? Like don't take it personally like is there just any advice that you have for just saying Maybe back up your arguments with facts instead of fake news Alternative Yeah, I don't know give some documentation and Make it a joint effort and Yeah, just try to argument your case as well as possible and then Maybe you can figure out if not you another thing you can do is bring in the second Opinion by someone else Who's not opinionated or biased? That's something else you can do does that answer your question. Yeah, okay More questions in the other room Hi, hi Sorry You want some water? Thanks Is there In terms of reviewers are there a is there an optimal amount of reviewers? Yeah, are they too many review? Can you have too me too many? Yeah So I'm concerned about the feedback loop, right? So you have five reviewers on on a pull request One says yes. Yeah, the second one says yes third one says No, let's do some exactly. Let's do some refactoring on this. It goes back into that cycle again Yeah There's some projects where we do one reviewer there's some projects where we do three reviewers depends on the client depends on projects and I Notice that if you have three reviewers or more The list of open pull requests only get longer and longer because some pull requests are being blocked by one person mostly me but Who has always has more arguments or things that need to change So I don't know if there's an optimum. I think that's something you have to figure out yourself for me I think two is It's pretty optimum Does that answer your question? Yeah, absolutely. I mean, yeah, there's no right answer to the right, but I'm just wondering if too many is But that's good. That's great. Thanks. All right more questions Yes Oh in the back. Hi. Oh, so can you not see me? That was it. That was a really great talk. I run a small team of developers And to just To make a point on the the lizard brain issue For the lady brought up over there the one way we've got round that lizard brain issue is We have like a what we call a code standard, but it's like basically just a code standard So it's just a basic list of rules that the team All agrees to so it's abstracted responsibility away from the individual code reviewer So for instance, like there's things like every method must have a comment. Oh, yeah So if my junior developer goes and submit something without a comment I just go heck to change that and there's no argument The team has agreed to that. Yeah, that's Something we do as well We have some regular discussions like right after this talk I have a call about Deployments and stuff and then we have a team discussion and then we make up some rules write them down and then when And someone news comes in someone new comes in They have to read all the rules which is a bit cumbersome, but yeah I'm always referred to that when we have discussion again about something. So my actual question is Would you agree that like a code review isn't about the here and now? That you can't it's not about improving code right now because essentially if you subscribe to the idea that all code is terrible Which it generally is You can't you can't really fix it at that precise moment What it's about that the code review process is about what you touched on to begin with which is the getting the team to start talking to each other and start learning from each other and Indirectly that should increase the quality of code over time But like an individual review won't fix much more than the missing comment or something like that. Yeah, correct I agree. Oh, sorry. Was there a question? That was kind of wondering if that's what you agree? I agree. Okay. I Agree, I Agree, there's too many fake news All right more questions Yes, I see two more hands Earlier you talked about how you browse through atomic commit sometimes. What are your thoughts on that versus squashing a whole merge request all the commits together Because sometimes with like a large code base if you're going back in time We want to know why it changes made it's easier when I only squash commits when I have something to hide Like a missing or a leftover of our dump or console the log then I squashed But otherwise I don't rewrite history It's always useful to see why something changed and then you can look at the commit message And then see the surrounding commits before and after so I am not a big fan of squashing commits But it's a personal thing. You can have a different opinion, of course Does that answer your question? Yeah, it does. Thanks. Mmm. I had one more Right. Hi. Sorry. We have a small team. So we never do code review actually like three of us But but we want to start what's the best Point to study like what's the best place to start code review? Do you use github or bitbucket? Yeah, we use github. Easy. Start using branches Yeah, what we do and then Start with small pull requests like changing a config file or something and then let someone else take a look at it Instead of pushing it straight to develop. Do you already use good branches? Yeah. Yeah, we have like staging You can also you know PR is basically a diff between two branches So you already have that you can already start right now start small with some small pull requests and then try to Do you have the phenomenon of Everlasting branches that are never being merged Like big branches that are a few months old and you can't get them merged We only we only have like free branches and just developer branch merged to staging and a staging goes to production Oh, okay, so each of us has his own developer branch Oh, and then it goes to one staging and then goes But we all have access to each other branch. Do you ever? Get into a situation where someone wants to continue working But also once their code being merged and then they make it the second develop personal develop branch, right? Yeah, because I want to You know how good works Yeah, you have and more commits and then this is the commit that you want to merge into develop But you want to continue working on the same progress, right? Then you can branch off from that again and then make that a pull request and continue working on something else Right We can have a discussion later. I have a call for work, but after that we can always have a discussion Cool. Thanks. I had to with some beers One more question. We have one more question two more Do you do any part of the code review? Offline just face-to-face or on the phone or something like that because we're a remote team. No But I can definitely see the value in that It's going back to the issue with the difficult bits. We tend to do the Criticizing bits offline and yeah, when it's so is it when something is crucial or important that you want to do a Add more developers to review a code to review Just trying to follow the sort of criticising private praise in public Routine so if there's something where yeah, this code is pretty bad Oh, I don't want to say that in front of everyone on get because if you Decouple the critiquing from the person that you're giving critique to then you can do public feedback instead of criticizing privately We never criticize privately actually Does that kind of answer your question or am I still on yeah, it's just interesting It's just we have a rule that if You say something and then the other person says no, I disagree should be how I've done it We just instantly have a rule to say okay. Let's take it offline I was just wondering if he had any kind of processes around that where someone disagrees with the critique Contrarily comment on that because it's a bit bragging but we have a great team so we don't have that situation Sorry, but everyone's every once a way of doing pull requests is different and every one's way of giving feedback is different So you can have different ways of doing it definitely. Thank you Do you have any standards about how much? Changes are updates are allowed into a pull request no hard rules or standards, but we do Give comments on pull requests that are too large or are dragging on too long So there's no hard rules for us, but we do notice them Do you have some tool or something to do that? No, but just just from experience when you and they're small It keeps things taken over then every now and again Huge one comes in and it was updated. You got to go back in you're wasting more time going in out in out So generally showing avoid them. I'm just curious from your own experience Sometimes I when you see the list of pull requests I reverse the list and see the oldest one first and then I go in and say is it still necessary if not close it if yes Require some action within one week or something That's something you can do as well So to reduce the list of holds inactive pull requests Does that answer your question? More one more. I think other people want to lunch My question is how would you solve the problem when there is a difference in opinion about the implementation, but not necessarily the code is bad or I Would bring in a third person To and have a team discussion about like he said Bring the team on par so that we all agree on some standards and some ways of doing things Of course, you don't have to Endlessly do endlessly do that and do it with every single little knit picking thing, but no, it's personal you can choose. I Don't have a proper way of answering that question. Is that We can still discuss afterwards more questions One more Do you have any advice on how to deal with people who can't help but be offensive with their comments? Then I would go privately and say maybe this comment is a bit snarky And maybe you should watch your language because you don't want other people to that to do that to you Maybe go higher up to the CEO or All right, I think we can have lunch now. Cool. Thanks