Video Summary and Transcription
The Talk discusses the code review process and the importance of software quality. It emphasizes the need for maintainability in code and the use of guidelines tailored to the team. The Talk also highlights the significance of functional suitability and the challenges of code review. Automation and documentation are recommended to improve code reviews and ensure software quality.
1. Introduction to Code Review Process
I'm here today to share what I've learned from researching the most used JavaScript libraries in GitHub. Let's understand the best practices for code review. The code review process involves creating code, opening a pull request, and having teammates review and provide perspectives. Devs review code based on standards, readability, and whether it functions correctly. Ideas for code review come from experience and can be empirical. Software testing uses the ISO 25010 standard.
So, I'm very... It's a pleasure to be here today with you guys. Thank you for all the organization that makes this happen and, definitively, it's an honor to be here today. This is my first time speaking here in Europe, so, it's another memoir, moment in my life and my career. And I hope you guys can enjoy this talk as well.
So, I was already introduced, so I will save you guys from hearing this again. The idea here in this talk is to share with you what I've learned after doing research on ten of the most used JavaScript libraries in GitHub. And I try to structure this in a way that we can basically understand a little bit more about the best practices that these guys have been using when reviewing code. And that is the approach. The idea today is not to go on the features that the libraries have, but instead is to learn more about how they are reviewing the code that are submitted to these repositories.
So, let's move ahead and think a little bit more about the code review process, right? The code review process is basically a simple way for people that are coding every day, right? It's something that is pretty straightforward, but for the people that are in the testing side, sometimes doesn't know exactly how it works. So, first, we have the code that is created. So, the developer goes there and creates a piece of code, or even the testers, depending on how you guys are structuring the way that you produce your test automation code, right? But this is the first part to create a code. Then the person that created this code opened APR. And after opening APR, that's the pull request, basically we have our teammates going there and reviewing this and providing us some perspectives, the views that they have. And then after that, we can, if we have the approve, we can go ahead and merge the code. If not, we need to work on this pull request, or in this piece of code that we are producing, right? The thing is that there is a very, how to say, very important activity that happens during this process that is basically to have these teammates looking at the code and reviewing that. The question is, how do devs do it? And the answer is magic, right? Actually, let me ask you guys that are devs. Tell me what do you have in mind when you are reviewing a code? That's your turn. Yeah, we're doing standards, right? What else? That's a very good question. That's a very good answer as well. Readability. What else? Yeah, if the code does what it should do, right? However, where do these ideas come from? Yeah, experience, right? The way that you do it. There's no, I'm not telling you that everyone does it this way, right? But sometimes we do it from an experience manner, right? Based on our experience. So, it makes us to be, how they say, empirical, right? So, if everyone here raised their hand and shared one thought about how to review, we would be the wisest person in this whole conference, right? There is another way to do it. In software testing, we use a standard called ISO 25010. Have you guys heard about it? Yeah? Raise your hands. Okay, good. Do a wave. We got thumbs up.
2. Software Quality and Code Review
There are eight software quality characteristics, including functional suitability and maintainability. Maintainability focuses on making the code readable, testable, and better. In a research study, 75% of comments on pull requests were related to maintainability. To understand how software testing repositories handle code reviews, I analyzed closed pull requests. I encourage you to try this research method in your company to gain insights into quality.
Okay, good. So, there are basically eight software quality characteristics. If the software is working as expected, it is classified as functional suitability. If the code is readable, it is called where is it? Okay, maintainability. Okay? There are groups of things. And what happens is that every time that I ask a developer on what they think that they should do during the review, what I heard more commonly, more frequently is related to maintainability.
Okay? It's making the code readable. It's making the code testable. It's making the code better. That's all about maintainability. I did a quick research on one of the teams that I was working with. And I reviewed 52 pull requests and more than 300 comments on the pull requests. What I learned? 75% of the comments were related to maintainability.
Okay? The thing is, how the most used software testing repositories, software testing libraries, how they say, do when they are when the people that contribute to this project are reviewing pull requests. This is the question that I had. And then I started to find out the answers. And what I did was basically to one, go one by one of these repositories and then filter only the closed pull requests.
Okay? Because they are already, how they say, established. Everything that should take as a decision was already did. Right? That is what I did. And I recommend to you guys to remember this slide. Because I really would like to have you trying to do this in your companies. At least for one repository. And I promise you, you will learn a lot about how you guys understand quality in that specific repository.
Okay? So, try to reproduce this research method. So, second was to open the pull request that has comments. Okay? Because sometimes we have pull requests that are just approved. Magically. Okay? They are just approved. And then ignore the pull requests that have only bot interactions or the own author.
3. Pull Request Monologues and Repository Insights
Sometimes authors open pull requests and have internal monologues before finding a more readable way. Ignore this and seek perspectives from others. Read comments and classify them against ISO 25010. Analyzing ten pull requests, I found some interesting repositories: Jest, Storybook, Mocha, Cypress, Puppeteer, Selenium, Jasmine, Vitest, and WebDriver.io.
Because it happens as well. It's something that is, how they say, very funny. And I just discovered that after doing this research.
Sometimes the author goes there and opens the pull request and starts to talk to himself. You know? Like, oh, I did it because I found this very interesting. And then two minutes after he said, oh, I found a more readable way to do it. It's like a monologue. Right? It's very interesting. So, you need to ignore this. Because then you want to have the perspective from others. Right? That's the magic around the pull request.
And then the fourth one is read the comments and try to classify this comment against the ISO 25010. Just to understand how you guys are understanding quality related to the pull requests. This is what I did to this ten pull requests.
And what I discovered over the ten projects that I think that I was very fast here. Just getting back to give you guys the opportunity to understand what were the repositories. So, Jest, Storybook, Mocha, Cypress, Puppeteer, Selenium, Jasmine, Vitest, I promise you, I come to say Vitest. And then I was like, okay. The previous talk said it's not Vitest. It's Vitest. Okay. Learned. And WebDriver.io. Is there any of these repositories that is very new to you? Raise your hand. Which one? WebDriver. Oh, gosh. It's really or it's a joke? No? It's real. So, WebDriver is one of the precursor for the web test automation. The frontend web test automation. That's why I'm making this a joke.
4. Maintainability and Guidelines in Code Review
WebDriver, Cypress, and Playwright are the most used JavaScript libraries. Maintainability is crucial in code review. Establish guidelines tailored to your team, not just industry standards. Use bots to check code quality, including file size, tests, and committers.
Basically, when I started, WebDriver was the Cypress that we have today, or Playwright, you know. Good. And then if you guys want to know more about how they were elected as the ten most used, you can visit 22.stateofjs.com. And then you have more information about them. Okay.
So, these are the most frequently used software quality characteristics from ISO 25010, based on the analysis that I did on this repository. Let's talk about maintainability. That basically is the ability that we have to evolve or to maintain the code. Okay? That is, again, the thing that is most used by the developers when they are reviewing code. So, I found many lessons learned about maintainability reviewing this pull request on this repository. However, these are the three that I decided to bring to you that I found very interesting, very important to have on your projects as well. Okay?
So, first is to establish guidelines. What I found in these repositories were that every time people were kind of fighting to have the guidelines applied. And it's important to remember that the guidelines is not only the general best practices, right? For example, if you see now that you have a person on your team that they are trying to modify, for example, a sequence of code and change and replace this by a future map and things like that. Maybe it doesn't apply to the code that you guys are doing in your team. So, establish your guidelines, not only what the market is pushing forth. This is what I learned about maintainability on these repositories. They really push to follow their guidelines. And some of the repositories also have a guideline page where you can learn more about it. Okay? So, you need to now think how we do this in our company. Because maybe these guidelines are only in your brain and then you are rejecting a lot of pull requests from your friends because it's not obeying your guidelines. So, make shared guidelines and it will be very useful for you as well.
So, second, use bots and linkers to check your code. Guys, I need to tell you this. I've never saw so much bots as I see during this research. You know, it's a lot. It's bots to verify the size of the files, the size of the production of a library that will be delivered. It's a bot to verify if there are flake tests. It's a bot to verify if the committer is John. Because we don't like John.
5. Code Review Tips and Functional Suitability
So, use these bots to help with pull requests. It's important to share knowledge and help new team members. Functional suitability is a key software quality characteristic. Write valuable tests and involve QAs for better coverage. Check the logic to avoid implementation mistakes.
So, we will reject all the pull requests that come from John. That's a joke. It's not true. But you got my point, right? So, use these bots, guys. They are there and they can help you guys to do this in a very useful way, okay?
Have a deeply technical team. Well, I saw a lot of comments where the guys were like, hey, don't use this method because this method will do two requests, two renders of this screen, you know? Or don't use this method because it will do three requests for this endpoint that you don't want to do it. And I was like, oh, gosh, if I was a developer here, how would I understand this to get this deeply knowledge about it, you know? And maybe as a junior developer, I would not know about it. So, how we as a team can help the people that is coming to our team to be better on it, you know? Maybe you have the senior guy sharing this kind of knowledge to the others or dedicating some time to study. I don't know. But this is one thing that I found very useful and helped a lot on this.
Second, functional suitability. This was the second software quality characteristic that was most used in this repository. So, I brought these three items here to discuss with you. First, write valuable tests. Valuable in this case means test that really makes sense to that piece of code that you wrote. Because sometimes we are just trying to do coverage, to increase the coverage, and it's not enough, right? Many times I saw people saying, hey, you forgot to cover this ad case or that ad case. So, it's important to not go and just do the happy path, but also think about these other scenarios, right? And here's my tip to you. If you have a QA around you, okay? There are QAs here, right? Oh, good, good. So, take these guys. Bring them close to you and say, hey, buddy, do you see other things that I need to test here? You see? Or also, ask them to teach you test design techniques. Test design techniques are like algorithms on how to read a piece of source and define what to test. That's incredible, okay? So, test design techniques. Second, check the logic. Because if you don't check the logic, maybe you are implementing this in the wrong way. I found many comments saying, hey, you implemented this logic in the wrong way. And sometimes I saw also comments saying, hey, you implemented this in the proper way. However, you implemented it in the wrong file. Yeah. It's not a joke. It's true.
6. Code Review Challenges and Performance
In open source projects, many people submit pull requests, causing various issues. Running tests is crucial to avoid broken pull requests. Performance improvements include using cache, improving loading, and removing unnecessary requests. Security comments relate to libraries with vulnerabilities. Usability, reliability, compatibility, and portability are often overlooked in code reviews.
I really saw this happening. But in an open source project, it may happen a lot. Because, you know? So, I remember that I was talking to a friend and he said, oh, I submitted a PR to that special project and they approved my PR. Yeah, look at me. Do you want to take a picture of me? Thing like this. You know? So, yeah. There are a lot of people trying to submit things and then it cause a lot of other issues, right?
So, third, run the tests. And again, it may sound cliche, but you need to run the tests that you already have. Sometimes I saw pull requests being rejected because the tests were broken. How did you open a pull request without running the test? Do you know what I mean? It's very important as well.
Performance. The first thing that is the most frequent mention by the developers were, maybe you can use cache here. Because you are requesting this file many times and they are not changing so much. So, you can use this and it will definitely help the performance. So, second, improve the loading. It's more related to packages that really are how to say, providing a tool behind the code, right? I mean, like Cypress, for example. You have the the graphical user interface. And if there is a possibility to improve the render velocity, maybe you can invest your time there as well. I saw some comments related to this. And the final one here in performance is remove the unnecessary. Sometimes we are requesting things many times and we just need to do this one time. So, this is something that is really common, really frequent in the comments that I saw as well. Okay. And then security. I wasn't able to find some, how to say, some more security comments, but the ones that I found was related to it. Basically, libraries that have some vulnerabilities and then the team need to talk to each other and see how they are going to deal with it. The last user quality characteristics were this. Usability, reliability, compatibility, and portability. And what this means. This means that sometimes we are forgetting to approach this kind of things during the code reviews and then someone is testing this for us.
7. Code Review Takeaways
Shift quality characteristics left during code reviews to find more quality on your project. Research your PRs to create checklists and anticipate mistakes, saving time and effort. Have a shared test strategy across the team to distribute tests and save time and effort.
If you had the QA team and the QA team has time enough to go there and to test it, it will be very good. But if they are not, who will test this for you is your client, right? So, that's the point here. Some takeaways to you guys.
First one, shift quality characteristics left, okay? Probably you guys heard a lot about shift left testing, right? But shift the software quality characteristics left. And how you do that? You start to, how they say, on purpose, during your code reviews, ask yourself about these eight software quality characteristics. Okay. And if you have questions, talk to your QA. If you don't have a QA, search on Google and find some information about how to do it. But if you are able to do it, to shift this left, you will be able to find out more quality on your project.
Second, research your PRs. It's basically to run this research on your company. Learn more about how your developers think and then try to create some checklists about it. Because then if you have this checklist, you'll be able to anticipate these mistakes. Every time that we submit the PR, there are lots of people working on it, time running, right? So, if you are able to send the APR that is more, how they say, proper, you will save a lot of time and a lot of effort. Okay.
And the third one is have a shared test strategy across the team. So, if the QAs created a test strategy, they are covering lots of tests. Sometimes we have, for example, a test strategy that has 50 tests. But the QAs, sometimes they don't have this deep technical knowledge to understand that 25% of those 50 tests can be done in the unit layer. And they are doing that on the UI layer, losing a lot of time. They know wasting a lot of time. If you talk together, you can distribute these tests. And then you guys will be able to save a lot of time and a lot of effort. Okay?
So, let me get back just a second. Almost there. Okay. Now it's time to say muito obrigado. That means thank you so much, guys. It was a pleasure to be here today. And I hope you enjoyed my talk.
Code Review Automation and Hotfix Balancing
Using static code analysis and automated tests before requesting code reviews is important and valuable. Automating this process through CI saves time and ensures maximum value. When balancing hotfixes with software quality, a risk-based analysis can help determine which processes can be skipped without compromising delivery. Encouraging maintainers to follow the eight principles of code quality requires further study.
Thank you. We've had, honestly, more questions again than we'll have time for. But I have got a few for you. Firstly, what are your thoughts about using static code analysis and automated tests and then only asking for a code review once they pass? Yeah. I think that this is definitely something that is important and valuable. I recommend a lot. And this is something that I saw in the comments as well. But what happened is that if you are able to put this static code analysis in the CI, then it's easier and it will save time from your team, right? Every time that you defer something to a human being to do it manually, I mean, go there and to run manually the static analysis, it can be skipped, forgot. And you guys are not going to extract the most value on it. Absolutely. I think that's a lot of automation is around being most effective with the time and expertise that you have as an individual in a team.
Another question is, and this one has a ton of upvotes, I suppose, likes. How do you recommend or how have you seen teams follow guidelines for hotfixes, where they really do have to go live as quickly as possible and then you have to balance all of these eight qualities versus just fixing whatever the issue is? Yeah, this is difficult. What I recommend, guys, is that every time that you have a... I will not talk only about hotfixes. I will talk about everything that you have when coding, right? Try to use risk-based analysis. That is basically to say, hey, what are the risks that we have to not deliver this now or to deliver this as it is? Because if you do this risk analysis, you'll be able to say, hey, can we skip that specifically process now in order to delivery? Is this good enough to deliver now or we will need to literally wait a little bit more before to deliver? Do you know what I mean? So this risk-based analysis is something that has been helping me and the projects that I work for as well. So in this way, we can select which one of those eight software quality characteristics are the better to the delivery that we need to do in this hotfix or whatever.
Excellent. Thank you. And that makes sense. Oh, there's so many. They're coming in quicker than we can answer them. This is great, though, and a great moment to just remind you that every speaker at the end of our Q&A will head over to the speaker questionnaire near the entrance. So we will have more time to answer questions then. So, oh, so many to pick from. So your study looks at what people currently do in code reviews to maintain these kind of eight principles of code quality. Did you look at what the right thing to do might be for maintainers in order to encourage those eight properties? So I think I didn't get the... So you took a look at the top 10 projects and you looked at what people did. Have you, or do you have any thoughts around what people did and what people could do or should do in order to improve quality are different? Yeah, I got you.
Code Review Insights and Documentation
To effectively apply code reviews, it's important to consider the skills of the people involved, not just the code itself. Conduct research in your own company to identify areas for improvement in security, performance, and reliability. If these aspects are neglected, it can lead to issues when the code is already in production. When documenting coding guidelines, learn from projects with many contributors and popular libraries. This will provide valuable templates and insights to support your work.
Yeah, I think that in order to have this answer, I would need to talk to the people in the projects, you know, because it's not only about the code, it's also about the skills that these people have. For example, if I ask you guys to raise your hand now, the person, the people that knows a lot about security, for example, raise your hands just to have a better notion. One person, do you know? So if we don't have this kind of knowledge, it's difficult to apply this on code reviews. That's why I'm encouraging you to do the same experiment, this research on your companies. And if you detect that you are not investing much time on security, on performance, on reliability, then you have a path of what to study in the next days in order to bring this back. Because, again, if you don't do this, someone will do. And if it's your client, it will not be good, right? Because it will already be in production. That's the point.
Excellent. And I know my job is to keep this to time and the time it just hits, I'm going to squeeze one really quick one in. But if I may, do you have any recommendations on how to then document coding guidelines in order to support contributors? Yeah. What I would do if I need to write a guideline now is to go to these main projects that has a lot of contributors and see how they are doing. So I definitely recommend you guys to take a look on the first three in the list because they are the most used, you know? And then you will see a lot of good templates and good ways to do it. So we will be able to also have a way to base your work the work that you are going to do. Excellent. Thank you so much. Once again, speaker question area near the front. But can we please have a huge round of applause for what was a really, really interesting talk.
Comments