Buổi 15: AI Code Review Tự Động Trên PR
Mục Tiêu Buổi Học
Sinh viên sẽ xây dựng GitHub Action tự động review code trên PR, phát hiện code smells (magic numbers, bad naming, DRY violations) và bắt buộc dev fix bằng evidence — không phải chỉ đồng ý.
Phần 1: Lý Thuyết (20 phút)
cm-code-review: Full Review Lifecycle (3 Parts)
Code review không phải task một chiều. Nó là interactive workflow với 3 phases rõ ràng:
Part A: Request Review
- Dev hoàn thành feature → push PR
- Gọi reviewer (AI hoặc human)
- Reviewer phải check đầu vào: có test không, architecture có hợp lý không
Part B: Receive Review
- Reviewer tìm issues: bugs, code smells, violations
- Post comments trực tiếp trên PR với evidence (line number, code snippet, reason)
- Dev phải respond — không phải chỉ đồng ý mà phải verify
Part C: Finish Branch
- Dev fix issues
- Dev re-request review
- Reviewer approve
- Merge PR
- Clean branch
Diagram Lifecycle:
┌─────────────────┐
│ Dev push PR │
└────────┬────────┘
│
Part A: Request
│
▼
┌─────────────────┐
│ AI Review │
│ Post comments │
└────────┬────────┘
│
Part B: Receive
│
▼
┌─────────────────┐
│ Dev responds │
│ with evidence │
└────────┬────────┘
│
Part C: Finish
│
▼
┌─────────────────┐
│ Approve + │
│ Merge │
└─────────────────┘Anti-Pattern: Performative Agreement ❌
BAD RESPONSE (Performative Agreement):
Reviewer: "This validates email without checking for SQL injection."
Dev: "Good catch! Fixed." ❌Why bad?
- Dev didn't actually verify
- Might not have fixed correctly
- Creates false sense of security
- No learning happens
GOOD RESPONSE (Evidence-Based):
Reviewer: "This validates email without checking for SQL injection."
Dev: "I verified this: [evidence]" ✅
- Added SQLite parameterized query (line 47)
- Changed from: user.insert(email)
- To: db.prepare('INSERT INTO users (email) VALUES (?)').run(email)
- The suggestion is correct because SQL injection on email field could expose user_id column
- Added unit test: test/security/sql-injection.test.js
- Fixed." ✅Why good?
- Dev showed they understand the problem
- Dev verified the fix with evidence
- Dev explained the risk
- Dev added test coverage
- Creates learning opportunity
Response Framework: 3 Types
Khi reviewer comment, dev respond theo framework:
| Situation | Response |
|---|---|
| Technically Correct | Fix it immediately. Acknowledge the issue. |
| Unclear Intent | Ask clarification: "Do you mean...? Can you point to example?" |
| Technically Questionable | Challenge with evidence: "I think this is safe because... Can you explain the risk?" |
| Stylistic Preference | Discuss trade-offs: "This is more readable but slower by 5%. Trade-off worth it?" |
cm-clean-code: 7-Point Hygiene Checklist
AI reviewer will scan for:
1. Dead Code → DELETE don't comment
// ❌ BAD
// const oldHandler = (req, res) => { ... } // Not used anymore
// ✅ GOOD
// Removed oldHandler. Logic moved to newHandler (commit xyz)2. Unused Imports → REMOVE
// ❌ BAD
import axios from 'axios';
import { readFile } from 'fs';
import moment from 'moment';
export const getUser = (id) => {
// Only uses fetch, never uses axios, readFile, or moment
}
// ✅ GOOD
import { readFile } from 'fs';
export const getUser = (id) => {
// Removed unused axios, moment
}3. Magic Numbers & Strings → EXTRACT to named const
// ❌ BAD
if (email.length > 255) throw new Error('Email too long');
if (password.length < 8) throw new Error('Password too short');
if (attempts > 5) lockAccount();
// ✅ GOOD
const VALIDATION = {
EMAIL_MAX_LENGTH: 255,
PASSWORD_MIN_LENGTH: 8,
LOGIN_MAX_ATTEMPTS: 5
};
if (email.length > VALIDATION.EMAIL_MAX_LENGTH) throw new Error(...);
if (password.length < VALIDATION.PASSWORD_MIN_LENGTH) throw new Error(...);
if (attempts > VALIDATION.LOGIN_MAX_ATTEMPTS) lockAccount();4. Naming → RENAME to describe intent
// ❌ BAD
const x = userData.filter(u => u.age > 18);
const p = x.map(u => u.password);
// ✅ GOOD
const adultsOnly = userData.filter(user => user.age > 18);
const passwordsOfAdults = adultsOnly.map(user => user.password);5. Single Responsibility Principle (SRP) → EXTRACT into separate units
// ❌ BAD — validateUser does 4 things
const validateUser = (user) => {
// 1. Check email format
// 2. Query database
// 3. Log to file
// 4. Send email notification
}
// ✅ GOOD — Separate concerns
const isValidEmail = (email) => { /* check format */ };
const userExists = (email) => { /* query database */ };
const logValidation = (email, result) => { /* log to file */ };
const notifyUser = (email) => { /* send email */ };6. DRY — EXTRACT shared logic
// ❌ BAD — Duplicate validation in 3 routes
app.post('/register', (req, res) => {
if (!req.body.email) return res.status(400).json({ error: 'Email required' });
if (req.body.email.length > 255) return res.status(400).json({ error: 'Email too long' });
// ...
});
app.put('/users/:id', (req, res) => {
if (!req.body.email) return res.status(400).json({ error: 'Email required' });
if (req.body.email.length > 255) return res.status(400).json({ error: 'Email too long' });
// ...
});
// ✅ GOOD — Extract to middleware
const validateEmail = (req, res, next) => {
if (!req.body.email) return res.status(400).json({ error: 'Email required' });
if (req.body.email.length > 255) return res.status(400).json({ error: 'Email too long' });
next();
};
app.post('/register', validateEmail, (req, res) => { /* ... */ });
app.put('/users/:id', validateEmail, (req, res) => { /* ... */ });7. Nesting Depth > 3 → EXTRACT, early return, guard clauses
// ❌ BAD — 5 levels of nesting
function processOrder(order) {
if (order) {
if (order.items) {
if (order.items.length > 0) {
if (order.customer) {
if (order.customer.verified) {
// Finally do real work at depth 5
}
}
}
}
}
}
// ✅ GOOD — Guard clauses, early return
function processOrder(order) {
if (!order) return;
if (!order.items || order.items.length === 0) return;
if (!order.customer || !order.customer.verified) return;
// Real logic at depth 1
chargeCustomer(order);
shipItems(order.items);
}Phần 2: Thực Hành (60 phút)
Task 1: Tạo GitHub Action Cho AI Review
File: .github/workflows/ai-code-review.yml
name: AI Code Review
on:
pull_request:
types: [opened, synchronize]
jobs:
review:
runs-on: ubuntu-latest
permissions:
pull-requests: write
contents: read
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0
- name: Setup Node.js
uses: actions/setup-node@v3
with:
node-version: '18'
- name: Install dependencies
run: npm install
- name: Get changed files
id: changed-files
uses: tj-actions/changed-files@v44
with:
files: |
src/**/*.js
!src/**/*.test.js
- name: Run AI Code Review
if: steps.changed-files.outputs.any_changed == 'true'
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
PR_NUMBER: ${{ github.event.pull_request.number }}
run: |
node scripts/code-review.js \
--files="${{ steps.changed-files.outputs.all_changed_files }}" \
--pr=$PR_NUMBERTask 2: Viết scripts/code-review.js
File này sẽ scan code violations từ 7-point checklist → post review comments trên PR.
Tạo file: scripts/code-review.js
const fs = require('fs');
const path = require('path');
const https = require('https');
/**
* AI Code Review for TaskFlow QA
* Scans for 7-point clean code violations
* Posts review comments to GitHub PR
*
* Usage: node scripts/code-review.js --files="src/api.js src/db.js" --pr=42
*/
class CodeReviewer {
constructor(config = {}) {
this.files = (config.files || '').split(' ').filter(Boolean);
this.prNumber = config.pr;
this.githubToken = process.env.GITHUB_TOKEN;
this.githubOwner = process.env.GITHUB_REPOSITORY_OWNER || 'your-org';
this.githubRepo = process.env.GITHUB_REPOSITORY?.split('/')[1] || 'taskflow';
this.comments = [];
this.violations = {
deadCode: 0,
unusedImports: 0,
magicNumbers: 0,
badNaming: 0,
srpViolation: 0,
dryViolation: 0,
nestingDepth: 0
};
}
/**
* Check 1: Dead Code — Comments that are NOT meaningful
*/
checkDeadCode(file, content) {
const lines = content.split('\n');
lines.forEach((line, idx) => {
const lineNum = idx + 1;
// Pattern: const x = ... // Not used
if (/^\s*\/\/\s*(const|let|var|function|class)/.test(line)) {
this.comments.push({
path: file,
line: lineNum,
side: 'RIGHT',
body: `🧹 **Dead Code** (Point 1)
This looks like commented-out code. Please delete it instead of commenting.
If you need to preserve history, git log has it.
✅ What to do:
- Delete the commented line(s)
- Or add a comment explaining WHY (e.g., "// TODO: remove after migration")
${line.trim()}`
});
this.violations.deadCode++;
}
});
}
/**
* Check 2: Unused Imports
*/
checkUnusedImports(file, content) {
const lines = content.split('\n');
const imports = new Map(); // { name: lineNum }
const usage = new Set(); // { name }
// Find all imports
lines.forEach((line, idx) => {
const importMatch = line.match(/import\s+(?:{([\s\S]+?)}\s+from|(\w+)\s+from)/);
if (importMatch) {
const imported = importMatch[1] || importMatch[2];
if (importMatch[1]) {
// Named imports: import { a, b } from 'x'
imported.split(',').forEach(name => {
const cleanName = name.trim().split(' as ')[0];
imports.set(cleanName, idx + 1);
});
} else {
// Default import: import a from 'x'
imports.set(imported, idx + 1);
}
}
});
// Find usage of imports
const importedNames = Array.from(imports.keys());
importedNames.forEach(name => {
const importLine = imports.get(name);
// Count usage outside import line
const usageCount = lines.reduce((count, line, idx) => {
if (idx + 1 === importLine) return count; // Skip import line itself
return count + (line.includes(name) ? 1 : 0);
}, 0);
if (usageCount === 0) {
this.comments.push({
path: file,
line: imports.get(name),
side: 'RIGHT',
body: `🧹 **Unused Import** (Point 2)
\`${name}\` is imported but never used.
✅ Remove this import:
\`\`\`diff
- import { ${name} } from '...'
\`\`\``
});
this.violations.unusedImports++;
}
});
}
/**
* Check 3: Magic Numbers & Strings
*/
checkMagicNumbers(file, content) {
const lines = content.split('\n');
// Pattern: if (x > 255) or if (attempts > 5) etc
const magicPatterns = [
{ pattern: />\s*(\d{2,})/, type: 'number' }, // > 100, > 255
{ pattern: /<\s*(\d+)/, type: 'number' }, // < 10
{ pattern: /(===|==)\s*['"](\w+)['"]/, type: 'string' } // == 'status'
];
lines.forEach((line, idx) => {
magicPatterns.forEach(({ pattern, type }) => {
if (pattern.test(line) && line.includes('if') || line.includes('while')) {
const magic = line.match(pattern)?.[1] || line.match(pattern)?.[0];
if (type === 'number' && magic && magic.length > 1) {
this.comments.push({
path: file,
line: idx + 1,
side: 'RIGHT',
body: `✨ **Magic Number** (Point 3)
The number \`${magic}\` should be extracted to a named constant.
✅ What to do:
At top of file, create:
\`\`\`javascript
const LIMITS = {
EMAIL_MAX_LENGTH: 255,
PASSWORD_MIN_LENGTH: 8,
LOGIN_MAX_ATTEMPTS: 5
};
\`\`\`
Then use:
\`\`\`javascript
if (email.length > LIMITS.EMAIL_MAX_LENGTH) { ... }
\`\`\``
});
this.violations.magicNumbers++;
}
}
});
});
}
/**
* Check 4: Bad Variable Naming
*/
checkBadNaming(file, content) {
const lines = content.split('\n');
// Single letter variables (except in loops)
lines.forEach((line, idx) => {
// Pattern: const x = or let d = or var p =
const badNameMatch = line.match(/(?:const|let|var)\s+([a-z])\s*=/);
if (badNameMatch && !line.includes('for')) {
const varName = badNameMatch[1];
this.comments.push({
path: file,
line: idx + 1,
side: 'RIGHT',
body: `📝 **Poor Variable Name** (Point 4)
Variable \`${varName}\` is too short. What does it represent?
✅ Better names:
- \`${varName}\` → \`users\` or \`data\` or \`timestamp\`?
- Use: \`const ${varName}Data = ...\` or \`const ${varName}List = ...\`
- Describe the **intent**, not the **type**`
});
this.violations.badNaming++;
}
});
}
/**
* Check 5: Single Responsibility Principle
*/
checkSingleResponsibility(file, content) {
const lines = content.split('\n');
// Look for functions that do too many things
// Pattern: long function with multiple unrelated operations
lines.forEach((line, idx) => {
if (line.includes('const ') && line.includes('=') && line.includes('=>')) {
// Get function body
let functionStart = idx;
let braceCount = 0;
let functionBody = '';
for (let i = idx; i < Math.min(idx + 20, lines.length); i++) {
functionBody += lines[i];
braceCount += (lines[i].match(/{/g) || []).length;
braceCount -= (lines[i].match(/}/g) || []).length;
if (braceCount === 0 && i > idx) break;
}
// Check for multiple operations (heuristic: many if/else, many function calls)
const ifCount = (functionBody.match(/if\s*\(/g) || []).length;
const callCount = (functionBody.match(/\.\w+\(/g) || []).length;
if (ifCount > 3 && callCount > 3 && functionBody.length > 500) {
this.comments.push({
path: file,
line: idx + 1,
side: 'RIGHT',
body: `🎯 **SRP Violation** (Point 5)
This function seems to do multiple things (${ifCount} conditionals, ${callCount} calls). Extract responsibilities.
✅ What to do:
1. **Identify responsibilities**: validation, database query, logging, response
2. **Extract**: Create separate functions
3. **Reuse**: Compose small functions
Example:
\`\`\`javascript
// Before: validateAndSave does 4 things
const validateAndSave = (user) => { /* 30 lines */ };
// After: Each function has ONE job
const isValidUser = (user) => { /* 5 lines */ };
const saveUser = (user) => { /* 3 lines */ };
const logUser = (user) => { /* 3 lines */ };
const notifyUser = (user) => { /* 3 lines */ };
\`\`\``
});
this.violations.srpViolation++;
}
}
});
}
/**
* Check 6: DRY — Don't Repeat Yourself
*/
checkDRY(file, content) {
const lines = content.split('\n');
const codeBlocks = [];
// Extract meaningful code lines (skip comments, blanks)
lines.forEach((line, idx) => {
const clean = line.trim();
if (clean && !clean.startsWith('//') && clean.length > 20) {
codeBlocks.push({ line: clean, num: idx + 1 });
}
});
// Find duplicates
for (let i = 0; i < codeBlocks.length; i++) {
for (let j = i + 1; j < codeBlocks.length; j++) {
const similarity = this.stringSimilarity(codeBlocks[i].line, codeBlocks[j].line);
if (similarity > 0.85) { // 85% similar
this.comments.push({
path: file,
line: codeBlocks[j].num,
side: 'RIGHT',
body: `🔄 **DRY Violation** (Point 6)
This code is similar to line ${codeBlocks[i].num}. Extract to shared function.
✅ What to do:
1. Create utility function
2. Call from both places
Line ${codeBlocks[i].num}:
\`\`\`javascript
${codeBlocks[i].line}
\`\`\`
Line ${codeBlocks[j].num}:
\`\`\`javascript
${codeBlocks[j].line}
\`\`\``
});
this.violations.dryViolation++;
break; // Only report first duplicate
}
}
}
}
/**
* Check 7: Nesting Depth > 3
*/
checkNestingDepth(file, content) {
const lines = content.split('\n');
let maxDepth = 0;
let maxDepthLine = 0;
lines.forEach((line, idx) => {
const depth = (line.match(/{/g) || []).length - (line.match(/}/g) || []).length;
// Cumulative depth
let currentDepth = 0;
for (let i = 0; i <= idx; i++) {
currentDepth += (lines[i].match(/{/g) || []).length;
currentDepth -= (lines[i].match(/}/g) || []).length;
}
if (currentDepth > 3) {
if (currentDepth > maxDepth) {
maxDepth = currentDepth;
maxDepthLine = idx + 1;
}
}
});
if (maxDepth > 3) {
this.comments.push({
path: file,
line: maxDepthLine,
side: 'RIGHT',
body: `📍 **Nesting Too Deep** (Point 7)
Nesting depth is ${maxDepth} levels. Keep it ≤ 3.
✅ Refactoring strategies:
1. **Guard Clauses** (early return):
\`\`\`javascript
// Before (depth 5)
if (x) {
if (y) {
if (z) {
doWork();
}
}
}
// After (depth 1)
if (!x) return;
if (!y) return;
if (!z) return;
doWork();
\`\`\`
2. **Extract Helper Functions**:
\`\`\`javascript
const isValidInput = (x, y, z) => x && y && z;
if (!isValidInput(x, y, z)) return;
doWork();
\`\`\``
});
this.violations.nestingDepth++;
}
}
/**
* Simple string similarity (Levenshtein-like)
*/
stringSimilarity(a, b) {
const longer = a.length > b.length ? a : b;
const shorter = a.length > b.length ? b : a;
if (longer.length === 0) return 1.0;
const editDistance = this.levenshteinDistance(longer, shorter);
return (longer.length - editDistance) / longer.length;
}
levenshteinDistance(a, b) {
const matrix = [];
for (let i = 0; i <= b.length; i++) {
matrix[i] = [i];
}
for (let j = 0; j <= a.length; j++) {
matrix[0][j] = j;
}
for (let i = 1; i <= b.length; i++) {
for (let j = 1; j <= a.length; j++) {
if (b.charAt(i - 1) === a.charAt(j - 1)) {
matrix[i][j] = matrix[i - 1][j - 1];
} else {
matrix[i][j] = Math.min(
matrix[i - 1][j - 1] + 1,
matrix[i][j - 1] + 1,
matrix[i - 1][j] + 1
);
}
}
}
return matrix[b.length][a.length];
}
/**
* Post all review comments to GitHub PR
*/
async postReviewComments() {
if (this.comments.length === 0) {
console.log('✅ No violations found! Code is clean.');
return;
}
const summary = `
## 🔍 AI Code Review Results
Found **${this.comments.length} issues** across 7-point hygiene checklist:
| Check | Count |
|-------|-------|
| 🧹 Dead Code | ${this.violations.deadCode} |
| 🧹 Unused Imports | ${this.violations.unusedImports} |
| ✨ Magic Numbers | ${this.violations.magicNumbers} |
| 📝 Bad Naming | ${this.violations.badNaming} |
| 🎯 SRP Violation | ${this.violations.srpViolation} |
| 🔄 DRY Violation | ${this.violations.dryViolation} |
| 📍 Nesting Depth | ${this.violations.nestingDepth} |
👉 **Fix these issues** and **re-request review** for approval.
---
`;
// Post summary as PR comment
await this.postPRComment(summary);
// Post individual review comments
for (const comment of this.comments) {
await this.postReviewComment(comment);
}
console.log(`\n✅ Posted ${this.comments.length} review comments to PR #${this.prNumber}`);
}
/**
* POST to GitHub API: Create PR comment
*/
postPRComment(body) {
return new Promise((resolve, reject) => {
const data = JSON.stringify({ body });
const options = {
hostname: 'api.github.com',
path: `/repos/${this.githubOwner}/${this.githubRepo}/issues/${this.prNumber}/comments`,
method: 'POST',
headers: {
'Authorization': `token ${this.githubToken}`,
'Accept': 'application/vnd.github.v3+json',
'Content-Type': 'application/json',
'Content-Length': Buffer.byteLength(data)
}
};
const req = https.request(options, (res) => {
if (res.statusCode === 201) {
resolve();
} else {
reject(new Error(`Failed to post comment: ${res.statusCode}`));
}
res.on('data', () => {});
res.on('end', () => resolve());
});
req.write(data);
req.end();
});
}
/**
* POST to GitHub API: Create review comment on specific line
*/
postReviewComment(comment) {
return new Promise((resolve, reject) => {
const data = JSON.stringify({
body: comment.body,
path: comment.path,
line: comment.line,
side: comment.side
});
const options = {
hostname: 'api.github.com',
path: `/repos/${this.githubOwner}/${this.githubRepo}/pulls/${this.prNumber}/comments`,
method: 'POST',
headers: {
'Authorization': `token ${this.githubToken}`,
'Accept': 'application/vnd.github.v3+json',
'Content-Type': 'application/json',
'Content-Length': Buffer.byteLength(data)
}
};
const req = https.request(options, (res) => {
res.on('data', () => {});
res.on('end', () => {
if (res.statusCode === 201) {
resolve();
} else {
resolve(); // Don't fail, continue to next
}
});
});
req.on('error', () => resolve());
req.write(data);
req.end();
});
}
/**
* Main workflow
*/
async run() {
console.log(`\n🔍 AI Code Review started...`);
console.log(`PR #${this.prNumber}`);
console.log(`Files to review: ${this.files.length}\n`);
for (const file of this.files) {
if (!fs.existsSync(file)) {
console.log(`⚠️ File not found: ${file}`);
continue;
}
const content = fs.readFileSync(file, 'utf-8');
console.log(`Checking ${file}...`);
this.checkDeadCode(file, content);
this.checkUnusedImports(file, content);
this.checkMagicNumbers(file, content);
this.checkBadNaming(file, content);
this.checkSingleResponsibility(file, content);
this.checkDRY(file, content);
this.checkNestingDepth(file, content);
}
await this.postReviewComments();
}
}
// CLI
if (require.main === module) {
const args = process.argv.slice(2).reduce((acc, arg) => {
const [key, value] = arg.split('=');
acc[key.replace('--', '')] = value;
return acc;
}, {});
const reviewer = new CodeReviewer(args);
reviewer.run().catch(err => {
console.error('❌ Error:', err);
process.exit(1);
});
}
module.exports = CodeReviewer;Task 3: Demo — Create PR with Code Violations
Step 1: Create feature branch
git checkout -b feature/user-validationStep 2: Write code with violations
File: src/api.js (with intentional violations)
// ❌ Violation 1: Unused import
import axios from 'axios';
import moment from 'moment'; // unused
// ❌ Violation 2: Bad naming
const x = [];
// ❌ Violation 3: Magic numbers
const validateUser = (user) => {
if (user.email.length > 255) throw new Error('Too long');
if (user.password.length < 8) throw new Error('Too short');
// ❌ Violation 4: SRP — does validation + logging + notification
if (user.age > 18) {
console.log('User age: ' + user.age);
sendEmail(user.email);
}
};
// ❌ Violation 5: Deep nesting
const processOrder = (order) => {
if (order) {
if (order.items) {
if (order.items.length > 0) {
if (order.customer) {
if (order.customer.verified) {
// Finally do work at depth 5
}
}
}
}
}
};Step 3: Commit & Push
git add .
git commit -m "feat: add user validation"
git push origin feature/user-validationStep 4: Create PR on GitHub
Go to GitHub → Create Pull Request
Step 5: Watch AI Review
- GitHub Actions runs
- AI Code Review workflow triggers
- Posts violations as comments:
- Line 2: "Unused import
axios" - Line 3: "Unused import
moment" - Line 5: "Bad variable name
x" - Line 8: "Magic number 255"
- Line 14: "SRP violation — does 3 things"
- Line 25: "Nesting depth 5 — too deep"
- Line 2: "Unused import
Task 4: Respond with Evidence
Dev Response to Comment #1 (Unused Import):
I verified this: [evidence]
✅ Fixed. Removed axios import at line 2.
- axios was added for future plan to call external API
- But we're using native fetch() instead
- Removed unused import
- Added comment in ROADMAP.md about why fetch not axios
The suggestion is correct because unused imports increase bundle size and reduce readability.Dev Response to Comment #5 (SRP):
I verified this: [evidence]
✅ Fixed. Split into 3 functions:
- isValidUser() — only validation (line 8)
- logValidation() — only logging (line 15)
- notifyNewUser() — only notification (line 19)
The suggestion is correct because:
1. Easier to test each function independently
2. Can reuse validation without logging
3. Can notify user without validation
Added unit tests: src/__tests__/api.test.js
- test/validation/valid-user → passes ✓
- test/validation/invalid-email → passes ✓
- test/notification/triggers-email → passes ✓📝 Quiz (5 câu)
- Full Lifecycle: Nêu 3 phases của code review workflow (Part A, B, C)
- Anti-Pattern: Tại sao "Good catch! Fixed." là performative agreement? Cách đúng là gì?
- Response Framework: Có 4 loại situation. Khi nào dev nên "challenge with evidence"?
- 7-Point Checklist: Nêu 4 violations trong code và cách fix
- GitHub API: Comment review dùng endpoint nào? (path, method)
🏠 Homework (30 phút)
- Create PR: Commit code với 3-4 violations
- Watch Review: AI posts comments
- Respond: Fix violations + respond with evidence (not just "fixed")
- Re-request: Re-request review
- Approve: Merge when approved
📚 Resources
- GitHub PR Comments API: https://docs.github.com/en/rest/pulls/comments
- Code smells: https://refactoring.guru/refactoring/smells
- Clean Code checklist: https://clean-code-developer.com/