Open Source Code Review Practices

TIPS FOR YOUR INTERNSHIPS AND NEW GRAD POSITIONS - 2024 EDITION There are some things you learn better once the roles are reversed: I learned the importance of a good pull request (PR) once I started reviewing them myself. Here is a checklist you can refer to: 1. Getting your work reviewed doesn't shift the responsibility of catching issues to your reviewers. The prime responsibility of ensuring your work is defect-free and won't cause problems in prod is always on you, the author. The code review process is a guardrail, but don't treat it as a crutch: 'I'll have a senior engineer review my work, so I don't have to worry about testing the edge cases, they'll catch those.' This is the wrong mentality to create a PR with. If all your PRs involve reviewers pointing out edge cases, you're not doing your job diligently. 2. Document your PRs properly. Provide context, and don't take this for granted. Just because someone reviews your PR doesn't mean they'll have the complete background. Include the WHAT, WHY, and HOW. WHAT: Provide background on the issue. Example: this PR fixes an uncaught exception (include details about the exception). WHY - Why is this fix necessary? Example: the fix is needed because it helps prevent the app from crashing unexpectedly because of the uncaught exception. HOW - Example: It's fixed by encapsulating this block of code within a try-catch and logging the error. 3. Add instructions on how to reproduce the error and verify the fix locally. 4. For UI changes, including screenshots of before and after can be helpful. 5. Add tests! You'd be surprised how often this step is forgotten. 6. Keep the PR small, so it's easy to review. The usual guideline is less than 250 lines of code per PR. If it's too large, break it down into multiple PRs. 7. Review it first yourself. You'd be surprised by how many print statements you'll find that you forgot to clean up. 8. Assign the right reviewers. 9. Call out things you want to bring specific attention to, and you can cc specific people. 10. You don't have to necessarily agree with every piece of feedback provided - if there's something you feel strongly about, feel free to discuss it. If the discussion is getting too long, consider switching to a different medium - my goto is to jump on a quick call. 11. Give people enough time to review large PRs. If you're planning on merging a big feature on Friday afternoon (which isn't a great idea to begin with), don't create the PR on Thursday evening. There can be exceptions to this rule, but rushed reviews should be avoided. In the worst case, keep in mind that your PR could be reverted, which is why keeping the PR detailed is necessary. Got any more suggestions? Drop them in the comments below! #softwareengineering #technology

100 lines of code: reviewed in 10 minutes. 1000 lines of code: reviewed never. Code reviews exist to catch bugs, improve maintainability, and help teams write better software together. But most engineers treat them like assignments to pass instead of collaborative checkpoints. That mindset kills the process before it starts. ➧ When you're submitting a PR: 1. Keep it small Aim for 10-100 lines of code per pull request. Past 100 lines, reviewers start skimming. Past 500, they stop caring entirely. Large PRs are harder to review, take longer to approve, and make it nearly impossible to catch real bugs. Break your work into isolated, logical chunks. Yes, it's more work upfront. But it ships faster. 2. Write a description Give context. Always. Your reviewer might be on a different team, in a different timezone, or new to the codebase. Don't make them guess what you're solving. If you're fixing a bug, explain what broke and link to the ticket. If it's a visual change, add before/after screenshots. If you ran a script that generated code, paste the exact command you used. Context turns a confusing diff into a clear story. 3. Leave preemptive comments If part of your diff looks unrelated to the main logic, explain it before your reviewer asks. "Fixed a typing issue here while working on the main feature." "This file got reformatted by the linter, no logic changes." These small clarifications save back-and-forth and show you're thinking about the reviewer's experience. ➧ When you're reviewing a PR: 1. Be overwhelmingly clear Unclear comments leave people stuck. If you're making a suggestion but don't feel strongly, say it: "This could be cleaner, but use your judgment." If you're just asking a question, mark it: "Sanity check, is this intentional? Non-blocking, just curious." Over-communicate your intent. Especially with remote teams or people you don't know well. 2. Establish approval standards with your team Decide as a team when to approve vs. block a PR. At Amazon and now at Nielsen, we approve most PRs even with 10+ comments because we trust teammates to address feedback. The only exception: critical bugs that absolutely can't go to production. Without clear standards, people feel blocked by style comments and approvals feel arbitrary. Talk to your team. Set the rules. Stick to them. 3. Know when to go offline Some conversations don't belong in PR comments. If the code needs a major rewrite, if there's a design disagreement, or if you're about to write a paragraph, stop. Ping your teammate directly. Have a quick call. Save everyone time. Leave a comment like "Let's discuss this offline" so they know you're not ignoring it.

In the last 11 years of my career, I’ve participated in code reviews almost daily. I’ve sat through 100s of review sessions with seniors and colleagues. Here’s how to make your code reviews smoother, faster and easier: 1. Start with Small, Clear Commits    - Break your changes into logical, manageable chunks. This makes it easier for reviewers to focus and catch errors quickly. 2. Write Detailed PR Descriptions    - Always explain the “why” behind the changes. This provides context and helps reviewers understand your thought process. 3. Self-Review Before Submitting    - Take the time to review your own code before submitting. You'll catch a lot of your own mistakes and improve your review quality. 4. Ask for Specific Feedback    - Don’t just ask for a “review”—be specific. Ask for feedback on logic, structure, or potential edge cases. 5. Don’t Take Feedback Personally    - Code reviews are about improving the code, not critiquing the coder. Be open to constructive criticism and use it to grow. 6. Prioritize Readability Over Cleverness    - Write code that’s easy to understand, even if it’s less “fancy.” Simple, clear code is easier to maintain and review. 7. Focus on the Big Picture    - While reviewing, look at how changes fit into the overall system, not just the lines of code. Think about long-term maintainability. 8. Encourage Dialogue    - Reviews shouldn’t be a one-way street. Engage in discussions and collaborate with reviewers to find the best solution. 9. Be Explicit About Non-Blocking Comments    - Mark minor suggestions as “nitpicks” to avoid confusion. This ensures critical issues get addressed first. 10. Balance Praise and Criticism    - Acknowledge well-written code while offering suggestions for improvement. Positive feedback encourages better work. 11. Always Follow Up    - If you request changes or leave feedback, follow up to make sure the feedback is understood and implemented properly. It shows you’re invested in the process. -- P.S: What would you add from your experience?

Shipping secure software in the age of open source + AI (a CTO’s friendly rant) I love open source. I like AI copilots. I also enjoy sleeping at night. Those can all co-exist if we treat security like a product feature, not a hope and a prayer. Here’s the playbook we use that keeps speed high and risk low: 1) Standards beat vibes AI can draft and OSS can accelerate, but our coding standards decide what ships. Keep them in-repo, short, and enforceable: auth, crypto, logging, errors, secrets, retries. Examples over essays. 2) “Do you understand this diff?” If you can’t explain a change in two sentences, it doesn’t merge. PRs must state intent, radius, data touched, and auth/perm impacts. Reviewers say what they tested and not “looks good.” AI-generated code still needs human brain cells. 3) Let robots be relentless Every commit runs Static Application Security Testing (SAST), dependency/Software Bill of Materials (SBOM) checks, license policy, and secrets detection. 4) Dynamic Application Security Testing (DAST) on deploy Spin up the env and point DAST at it: auth flows, input fuzzing, headers, cookies, rate limits. Block on criticals/highs; auto-ticket the rest. If staging can’t handle our scanner, it won’t handle theirs. 5) Pen tests by experts in the field For every major release, bring in an external pen test. Fix, retest, publish the delta. Fresh eyes > familiar blind spots. 6) Open source ≠ open season Curate approved packages and minimum versions. Generate an SBOM on every build and fail on banned/CVE’d deps. Watch for typosquats and weird transitive stuff (yes, we still remember left-pad). 7) Secrets: not in code, not in logs, not in CI Central secret store, rotation, short TTLs, least privilege. Friends don’t let friends .env in prod. 8) Threat model like adults (30 mins) New feature? List 3–5 abuse cases and one control each. Data class, authz paths, rate limits, input validation. Done is better than ornate. 9) Logs > vibes Security-minded logging (no PII dumps), trace IDs, anomaly alerts. Add a sane WAF and rate limits. At 3 a.m., “we think it’s fine” isn’t telemetry. 10) AI with seatbelts No secrets in prompts. Human review for anything touching auth, crypto, or persistence. Prefer vetted patterns over clever one-liners the model dreamed up. My “no-ship” gates ✅ Standards linted in CI ✅ PR intent + risk explained, reviewer confirms understanding ✅ SAST/Deps/SBOM/Secrets scans (fail on criticals/highs) ✅ DAST on deploy (block on criticals/highs) ✅ External pen test for every major change (with retest) ✅ Centralized secrets; no secrets in code/logs/CI ✅ Quick threat model per feature ✅ Telemetry + WAF + rate limits live and monitored Ship fast. Ship secure. Sleep better. And if a robot blocks your PR then thank it, fix it, and keep your weekend. ☕️🛡️ Want a one-pager you can paste into your pipeline? Happy to share. #AppSec #OpenSource #AI #DevSecOps #Security #SBOM #PenTest #DAST #SAST #CTO #bTrade