DEV Community

Timevolt
Timevolt

Posted on

The One Ring of Code Reviews: How a Single Practice Makes Your Code Legendary

The Quest Begins (The "Why")

I still remember the first time I opened a pull request that felt like stepping into Mordor. It was a “feature complete” PR that touched twenty‑seven files: a new API endpoint, a database migration, a handful of React components, a Redux slice, some CSS, and even a tweak to the Dockerfile. The description? “Add user auth”.

The reviewers stared at it like it was a dragon hoard. Comments piled up: “Why is this migration here?”, “Did you forget to handle the token expiration case?”, “This component looks… off”. After three days of back‑and‑forth, the PR finally merged… only to explode in production two weeks later because a edge‑case in the password‑reset flow had never been exercised.

That experience taught me a hard lesson: big, vague PRs are the silent killers of code quality. They make reviewers tired, hide bugs in plain sight, and turn every merge into a gamble. I wanted a better way—one that would change not just how I review code, but how I write it.

The Revelation (The Insight)

The treasure I found was surprisingly simple: keep every pull request small, focused, and self‑contained.

When you limit a PR to a single logical change—say, adding a database column or implementing one frontend button—you give reviewers a clear, bite‑sized problem to solve. They can actually read the diff, spot missing tests, question naming, and suggest improvements without drowning in noise.

But the magic doesn’t stop at review time. Writing small PRs forces you to think in increments. Before you even open the editor, you ask yourself: “What is the tiniest piece of value I can deliver right now?” That question nudges you toward better modular design, clearer abstractions, and—most importantly—automated tests that actually cover what you just shipped.

Wielding the Power (Code & Examples)

The Trap: A Monolithic PR

Imagine we’re building a simple “user profile” feature. The before version looks like this (all changes lumped together):

@@
@@ src/backend/models/User.js
-  const User = sequelize.define('User', {
-    id: { type: DataTypes.INTEGER, primaryKey: true, autoIncrement: true },
-    username: { type: DataTypes.STRING, allowNull: false },
-    email: { type: DataTypes.STRING, allowNull: false, unique: true },
-    passwordHash: { type: DataTypes.STRING, allowNull: false }
-  });
+  const User = sequelize.define('User', {
+    id: { type: DataTypes.INTEGER, primaryKey: true, autoIncrement: true },
+    username: { type: DataTypes.STRING, allowNull: false },
+    email: { type: DataTypes.STRING, allowNull: false, unique: true },
+    passwordHash: { type: DataTypes.STRING, allowNull: false },
+    bio: { type: DataTypes.TEXT, allowNull: true },          // <-- new column
+    avatarUrl: { type: DataTypes.STRING, allowNull: true }   // <-- new column
+  });

/backend/routes/profile.js
-  router.get('/:id', async (req, res) => {
-    const user = await User.findByPk(req.params.id);
-    res.json(user);
-  });
+  router.get('/:id', async (req, res) => {
+    const user = await User.findByPk(req.params.id);
+    if (!user) return res.status(404).send('Not found');
+    res.json({
+      id: user.id,
+      username: user.username,
+      email: user.email,
+      bio: user.bio || '',
+      avatarUrl: user.avatarUrl || ''
+    });
+  });
+ 
+  router.patch('/:id', authMiddleware, async (req, res) => {
+    const { bio, avatarUrl } = req.body;
+    const user = await User.findByPk(req.params.id);
+    if (!user) return res.status(404).send('Not found');
+    await user.update({ bio, avatarUrl });
+    res.json(user);
+  });

/frontend/components/ProfilePage.jsx
-  import React, { useEffect, useState } from 'react';
-  import axios from 'axios';
+  import React, { useEffect, useState } from 'react';
+  import axios from 'axios';
+  import AvatarUploader from './AvatarUploader';

-  export default function ProfilePage({ match }) {
-    const [user, setUser] = useState(null);
-
-    useEffect(() => {
-      axios.get(`/api/profile/${match.params.id}`).then(r => setUser(r.data));
-    }, [match.params.id]);
-
-    return (
-      <div>
-        <h2>{user?.username}</h2>
-        <p>{user?.email}</p>
-      </div>
-    );
-  }
+  export default function ProfilePage({ match }) {
+    const [user, setUser] = useState(null);
+    const [bio, setBio] = useState('');
+    const [avatar, setAvatar] = useState(null);
+
+    useEffect(() => {
+      axios.get(`/api/profile/${match.params.id}`).then(r => {
+        setUser(r.data);
+        setBio(r.data.bio ?? '');
+        setAvatar(r.data.avatarUrl ?? null);
+      });
+    }, [match.params.id]);
+
+    const handleSave = async () => {
+      await axios.patch(`/api/profile/${match.params.id}`, { bio, avatar });
+    };
+
+    return (
+      <div>
+        <h2>{user?.username}</h2>
+        <AvatarUploader avatar={avatar} onChange={setAvatar} />
+        <textarea value={bio} onChange={e => setBio(e.target.value)} placeholder="Bio" />
+        <button onClick={handleSave}>Save</button>
+      </div>
+    );
+  }

/frontend/components/AvatarUploader.jsx
+  import React, { useState } from 'react';
+ 
+  export default function AvatarUploader({ avatar, onChange }) {
+    const [preview, setPreview] = useState(avatar);
+   
+    const handleFileChange = e => {
+      const file = e.target.files[0];
+      if (file) {
+        const url = URL.createObjectURL(file);
+        setPreview(url);
+        onCallBack(file); // Oops! typo – will break later
+      }
+    };
+   
+    return (
+      <div>
+        <img src={preview} alt="avatar" />
+        <input type="file" accept="image/*" onChange={handleFileChange} />
+      </div>
+    );
+  }
Enter fullscreen mode Exit fullscreen mode

What a mess! The PR touches the model, backend routes, two frontend components, and even introduces a bug (onCallBack instead of onChange). Reviewers have to juggle mental models across the stack, and the chance of missing something skyrockets.

The Victory: Small, Focused PRs

Now let’s see the after—the same feature split into four tiny PRs, each with a clear purpose.

PR #1: Add bio & avatar columns to User model

/models/User.js
-  const User = sequelize.define('User', {
-    id: { type: DataTypes.INTEGER, primaryKey: true, autoIncrement: true },
-    username: { type: DataTypes.STRING, allowNull: false },
-    email: { type: DataTypes.STRING, allowNull: false, unique: true },
-    passwordHash: { type: DataTypes.STRING, allowNull: false }
-  });
+  const User = sequelize.define('User', {
+    id: { type: DataTypes.INTEGER, primaryKey: true, autoIncrement: true },
+    username: { type: DataTypes.STRING, allowNull: false },
+    email: { type: DataTypes.STRING, allowNull: false, unique: true },
+    passwordHash: { type: DataTypes.STRING, allowNull: false },
+    bio: { type: DataTypes.TEXT, allowNull: true },
+    avatarUrl: { type: DataTypes.STRING, allowNull: true }
+  });
Enter fullscreen mode Exit fullscreen mode

PR #2: Implement backend GET & PATCH endpoints

/backend/routes/profile.js
+  router.get('/:id', async (req, res) => {
+    const user = await User.findByPk(req.params.id);
+    if (!user) return res.status(404).send('Not found');
+    res.json({
+      id: user.id,
+      username: user.username,
+      email: user.email,
+      bio: user.bio || '',
+      avatarUrl: user.avatarUrl || ''
+    });
+  });
+
+  router.patch('/:id', authMiddleware, async (req, res) => {
+    const { bio, avatarUrl } = req.body;
+    const user = await User.findByPk(req.params.id);
+    if (!user) return res.status(404).send('Not found');
+    await user.update({ bio, avatarUrl });
+    res.json(user);
+  });
Enter fullscreen mode Exit fullscreen mode

PR #3: Create AvatarUploader component

/frontend/components/AvatarUploader.jsx
+  import React, { useState } from 'react';
+
+  export default function AvatarUploader({ avatar, onChange }) {
+    const [preview, setPreview] = useState(avatar);
+
+    const handleFileChange = e => {
+      const file = e.target.files[0];
+      if (file) {
+        const url = URL.createObjectURL(file);
+        setPreview(url);
+        onChange(file); // Fixed typo!
+      }
+    };
+
+    return (
+      <div>
+        <img src={preview} alt="avatar" />
+        <input type="file" accept="image/*" onChange={handleFileChange} />
+      </div>
+    );
+  }
Enter fullscreen mode Exit fullscreen mode

PR #4: Wire up ProfilePage to use the new API and component


diff
/frontend/components/ProfilePage.jsx
-  import React, { useEffect, useState } from 'react';
-  import axios from 'axios';
+  import React, { useEffect, useState } from 'react';
+  import axios from 'axios';
+  import AvatarUploader from './AvatarUploader';
+
-  export default function ProfilePage({ match }) {
-    const [user, setUser] = useState(null);
-
-    useEffect(() => {
-      axios.get(`/api/profile/${match.params.id}`).then(r => setUser(r.data));
-    }, [match.params.id]);
-
-    return (
-      <div>
-        <h2>{user?.username}</h2>
-        <p>{user?.email}</p>
-      </div>
-    );
-  }
+  export default function ProfilePage({ match }) {
+    const [user, setUser] = useState(null);
+    const [bio, setBio] = useState('');
+    const [avatar, setAvatar] = useState(null);
+
+    useEffect(() => {
+      axios.get(`/api/profile/${match.params.id}`).then(r => {
+
Enter fullscreen mode Exit fullscreen mode

Top comments (0)