As a C3.ai developer‚ reviewing other developers’ code teaches me about the applications C3.ai is developing and keeps me informed about new features in our products‚ code base‚ and tech stack.
In my experience as a developer‚ I have noticed that Pull Requests tend to be an area of little conversation when it comes to joining an organization. But Pull Requests are an essential part of continuing to add to the C3.ai codebase. Reading them across C3.ai gives me knowledge about what is going on across teams‚ shows me new ways to solve problems‚ and gives me a chance to both help other developers increase their skills as well as my own.
What is a Pull Request
A Pull Request is a tool to review a set of changes before they are added (merged) to the code base. The Pull Request tool lets developers only view what has changed on a per-file basis to review the content and accuracy of those desired changed.
Pull Requests are vital to ensuring the code going into our applications is well reviewed‚ maintainable‚ and held to a high standard. Here I will outline a few Pull Requests that exemplify the standards we hold C3.ai developers to.
Attention to Detail
A great Pull Request review addresses any issues with the logic in the code‚ typos‚ syntax‚ function clarity‚ and many other aspects of building solid‚ maintainable code. A good reviewer will notice ways the author could refactor or merge a function that already exists—like in this example:
Here‚ rsamp has suggested a way to merge the functions together and wangchy27 has gone back and merged her new function with the existing one to give future developers the ability to scroll an element on a page both vertically and horizontally without needing to call two functions.
Increased Efficiency
Another important part of code review is to be very thorough. It may seem like a pain at times but if developers do not hold everyone to the same standards and guidelines‚ the code quickly becomes inconsistent and no one knows what the real guidelines are. Also‚ a common mistake of younger developers is to trust seniority when it comes to reviewing their code. You may think they are right but we all make mistakes so review thoroughly and ask questions when you do not understand! We are all here to learn and get better. In the below example‚ the documentation for a function was not accurate for what the function was returning:
In this example‚ the @return documentation is saying to return a certain C3.ai Type but the function says to return a different type. This can confuse future developers who are expecting one thing and getting another. This is a common occurrence when we copy some existing file as the template for our new file and don’t update everything.
Improving Input
It should go without saying that developers are all on the same team‚ so make sure we are providing constructive criticism. In the below example‚ rsamp noticed a few lines of CSS that can be moved down to join a group of matching selectors. His comment is simple and straight to the point. This is much more polite than “Why didn’t you add these to the group at line 14‚” which has a more accusatory tone.
Lastly‚ a great code review is when a reviewer simply knows some of the caveats of the code base and can help inform a younger or less experienced developer. In the below example we learn that one application runs inside another application and the CSS changes are not inherited across the applications:
Here‚ rsamp happened to know the CSS changes must be made in a second file. This is a great example of knowledge sharing that other developers may come across later.
To wrap up‚ I want to encourage all developers to participate in more Pull Request reviews and continue to strive to make the C3.ai codebase one that is readable‚ maintainable‚ and full of great code!
