Code Review: How can we do it better?

💛CircleCI - Continuous Integration as a service
(Sponsored - use the link when signing up to support the channel)

🔗Pull Requests featured in the video

🔗 Follow on Twitch to get notified of live streams

🔗 Isa on Twitter

🔗 mpj on Twitter

🔗Support the show by becoming a Patron

🔗 Help translate the show to your language

💛 Follow on Twitch and support by becoming a Subscriber
We record the show live Mondays 7 AM PT

💛 Fun Fun Forum

💛 mpj on Twitter

💛 CircleCI (Show sponsor)
Robust and sleek Docker-based Continuous Integration as a service. I used CircleCI prior to them becoming a sponsor and I love that their free tier is powerful enough for small personal projects, even if they are private. Use this link when you sign up to let them know you came from here:

💛 Quokka (Show sponsor)
Wonder how MPJ evaluates JavaScript inline his editor. Quokka is the answer - use this link when you buy to let them know you came from here:

💛 FUN FUN FUNCTION
Since 2015, Fun Fun Function (FFF) is one of the longest running weekly YouTube shows on programming 🏅 thanks to its consistency and quality reaching 200,000+ developers.

🤦‍♂️ The Failing Together concept is what makes FFF unique. Most coding content out there focus on step-by-step tutorials. We think tutorials are too far removed from what everyday development is like. Instead, FFF has created a completely new learning environment where we grow from failure, by solving problems while intensively interacting with a live audience.

Tutorials try to solve a problem. Failing Together makes you grow as a developer and coworker.

📹 Each show is recorded live on Twitch in a 2-hour livestream on Mondays. The host, assisted by the audience, is tasked to complete a programming challenge by an expert guest. Like in the real world, we often fail, and learn from it. This, of course, reflects what the audience identifies with, and is one of the most praised aspects of the show.

⏯ On Fridays, an edited version of the show is adapted for and published on YouTube.

Content Topics revolve around: JavaScript, Functional Programming, Software Architecture, Quality Processes, Developer Career and Health, Team Collaboration, Software Development, Project Management

[Jhn.R]

a GOOD PR is a SMALL PR! Yes! Thank you! Finally someone said it!

[devlust]

I was expecting more tips on code review and actual code reviewing, but it seemed to me that most of the time you guys were explaining things totally not related to code review :/
Might be just me, but I really don't like when you have a topic for something and you spend more time doing stuff not related to the topic, feels like wasting time.
But also I do understand that this was a live stream and it's just how things naturally go. Anyway it's just a tip on focusing on the actual code review rather than explaining what an action is in the context of React for example :)
Keep up the good work, you're awesome!

[TheDevMethod]

I liked the comment about if there were unused variables that were removed in isolation from the purpose of the pull request, it’s probably a sign that your CI needs to be updated so those issues are caught earlier.

[dealloc]

At my work, I have made the analogy of PRs being your house. You wan't to clean up and label things as easily and concise as possible, so that your guests know how to go around the house and see your refurbishing. If it's all a mess, either guests will have a hard time getting through the door, but most often they just won't even go in.


If you think of PRs as a personal thing, that you need to take care of, I think it's a lot easier to reason about and understand the importance of.

[wallacesantos]

"Hey, my name is Isa".
Achei incrível como o inglês tava perfeito, até você falar seu próprio nome e falar perfeitamente em português, sem um pingo de sotaque, e deu pra notar que é brasileira kkkk

[MarcelRobitaille]

33:40 Another note about commented console.logs is that they can easily be changed to console.debug and left in. Like this, when debugging, they can easily be added to the filter of log messages.

[michaelc.]

General rules for prs:
1. Remember, you create a pr for a human, so please(!) make everything readable by human as much as possible: pr title, pr description, commit comments, and the most important - your code!!! You're writing code not for machines, not for yrslf, but for the other fellow people... Unless it's your pet project noone cares about :D
DRY KISS
2. See 1.

[sjaustirni]

Please do more code reviews. They are super interesting.

[ksauri]

This was super helpful!!! Was great to see accomplished programmers go through code, please put more of these on youtube

[FilipeDeschamps]

Isa!! Very nice introduction, I'm LMAO 😂

[MrMisterkrazy]

Hold up one second, MPJ is working on private projects for D&D Beyond!? (GitHub screen at 4:22)

[wallacesantos]

I was expecting a .... *clap* *clap* CODE REVIEW

[ZefugiLive]

Wait a minute! When did Captain Jack Sparrow start coding ? ;D

[MarcelRobitaille]

53:41 I don't think this is the same. Doesn't this create a new function on every render, making it less efficient?

[nardoae]

You two are such a cute couple. I can't express enough gratitude for what you have both contributed to this community.

[TrvGuY]

I hope this question will still be read, given that the video is quite old already, but why do you pay attention to the individual commits of a PR (and their messages)? Personally I only look at the diff to the base branch (and the description of course), but not at the individual steps that resulted in that diff.

[kimjongun]

What's your opinion on passing anonymous functions vs named functions as argument?

[baldurpan]

I've managed to use linting to find commented out garbage.

So f.e. I setup eslint to make this ok:
// This is ok

But skipping the space after the fwd slashes gives an error
//Not ok

Then I simply skip the space when I'm commenting out things that shouldn't be commited. :)

[pauloferrato]

Here you can see Tony Stark teaching how he made Jarvis and Friday

[carlos_hb]

I try to enforce small PRs in my team but the team lead is like "Trying to separate changes just to keep PRs small adds extra effort I think is fine" and we end up reviewing +600 lines changed