1. Capisci il diff
- Riesci a spiegare in una frase cosa cambia per l’utente?
- I file modificati sono coerenti con il task?
- Sono comparsi refactor, rinominazioni o configurazioni non richieste?
- Ci sono file generati, database locali o dati personali inclusi per errore?
Prima della checklist: classifica il rischio della modifica
Non tutte le modifiche meritano lo stesso livello di review. Correggere un refuso e cambiare autenticazione non possono avere la stessa procedura. Prima di leggere il diff, classifica impatto e reversibilità: questo decide quanto approfondire test, sicurezza e verifica manuale.
Se il rischio è alto, chiedi una seconda review umana e prepara un rollback prima del merge. Se non sai classificare la modifica, trattala come rischio medio finché non hai chiarito dati coinvolti, utenti interessati e comportamento precedente.
- Rischio basso
- Testo, stile locale o modifica facilmente reversibile senza dati e senza logica applicativa. Serve comunque una verifica visiva o mirata.
- Rischio medio
- Componente, flusso o dipendenza che può creare regressioni circoscritte. Richiede build, test collegati e prova del percorso utente.
- Rischio alto
- Autenticazione, autorizzazioni, pagamenti, dati personali, migrazioni, cancellazioni o infrastruttura. Richiede review indipendente, test dedicati e piano di rollback.
2. Verifica il comportamento
- Il flusso principale funziona dall’inizio alla fine?
- Gli stati vuoto, loading ed errore sono gestiti quando servono?
- Il comportamento corrisponde agli acceptance criteria?
- Le funzionalità vicine continuano a funzionare?
3. Esegui i controlli tecnici disponibili
- Build di produzione.
- Test automatici collegati alla modifica.
- Lint o type check, se configurati.
- Controllo della console del browser.
- Verifica manuale del flusso realmente modificato.
Caso DottorLone: la build passava, il conto era sbagliato
Nella guida di questo sito sui preventivi era pubblicato un caso con 85 euro mensili, dodici mesi e 300 euro iniziali. Il totale indicato era 1.620 euro; quello corretto è 1.320 euro. L’errore cambiava la conclusione del confronto. La pagina statica, i metadati e la compilazione superavano i controlli: nessuno di quei controlli verificava il calcolo nel testo.
La correzione è stata divisa in due parti. Il caso editoriale ora usa due offerte complete e coerenti; il calcolatore esegue la formula in centesimi interi, con un test che mantiene anche il caso originale da 85 euro. Altri controlli verificano il pareggio a trenta mesi, il cambio di convenienza a trentasei e il rifiuto di dati non validi.
Il punto della review era formulare una domanda che la build non copriva: con questi input, il totale e il confronto sono corretti? Per un filtro sarebbe “quali elementi restano visibili?”. Per un modulo, “cosa succede dopo un errore?”. Il controllo utile dipende dal comportamento, non dal nome del comando.
Condizione: canone 85 euro, durata 12 mesi, avvio 300 euro.
Risultato pubblicato: 1.620 euro.
Risultato atteso: 85 × 12 + 300 = 1.320 euro.
Impatto: la conclusione economica dell'esempio si inverte.
Controllo mirato:
const totaleInCentesimi = 8500 * 12 + 30000;
assert.equal(totaleInCentesimi, 132000);
Da verificare separatamente:
- correttezza del testo e delle condizioni;
- calcolo con altri periodi;
- dati mancanti e input non validi;
- risultato visibile nel browser. Un pass di review ripetibile, dal diff alla prova finale
Una review utile lascia evidenze. Registra il comando eseguito, il risultato e ciò che non hai potuto controllare. “Sembra funzionare” non distingue una build superata da una semplice lettura del codice.
Adatta i comandi allo stack del progetto: quelli sotto sono segnaposto operativi, non istruzioni universali. Se il repository documenta comandi ufficiali, usa quelli.
1. Scope
Task: [obiettivo]
Non-goal: [cosa non deve cambiare]
2. Diff
File attesi: [lista]
File modificati: [lista reale]
Modifiche inattese: [nessuna / elenco]
3. Controlli automatici
Build: [comando] -> [esito]
Test mirati: [comando] -> [esito]
Lint/type check: [comando] -> [esito]
4. Verifica manuale
Percorso provato: [passaggi]
Viewport/stato: [desktop, mobile, errore, vuoto]
Esito: [risultato osservato]
5. Non verificato
[limiti, ambiente mancante, casi residui]
6. Decisione
[accetta / correggi / richiedi seconda review] 4. Controlla la UI reale
- Desktop e almeno una larghezza mobile.
- Testi lunghi, errori e contenuti vuoti.
- Navigazione con tastiera e focus visibile.
- Contrasto, etichette e messaggi comprensibili.
- Assenza di overflow, sovrapposizioni e controlli irraggiungibili.
5. Cerca rischi di sicurezza e privacy
- Nessuna API key, password o connection string nel frontend o nel diff.
- Gli input non vengono inseriti direttamente in HTML o comandi pericolosi.
- Autorizzazioni e controlli non sono affidati soltanto all’interfaccia.
- Log ed errori non espongono dati sensibili.
- File locali e configurazioni private restano fuori da Git.
6. Valuta nuove dipendenze
- La libreria risolve un problema reale che il progetto non gestisce già?
- È mantenuta e compatibile con lo stack?
- Quanto aggiunge al bundle e alla superficie di rischio?
- È stata aggiunta solo perché l’agente la conosceva meglio?
Prompt copiabile per la review finale
Fai una review finale delle modifiche locali.
Controlla in questo ordine:
1. Bug e regressioni rispetto al task.
2. Modifiche fuori scope.
3. Segreti, dati sensibili e configurazioni locali.
4. Stati errore, vuoto e loading.
5. Responsività, accessibilità e testi lunghi.
6. Dipendenze nuove o non necessarie.
7. Test e verifiche mancanti.
Riporta:
- finding ordinati per gravità
- file e righe coinvolte
- controlli eseguiti davvero
- rischi residui
Non dichiarare eseguito un test che non hai potuto lanciare. Come scrivere un finding che si possa correggere
“Questo codice non mi convince” non aiuta chi deve intervenire. Un finding azionabile collega condizione, comportamento, impatto e posizione. Quando possibile aggiunge il controllo che dimostra il problema.
Ordina i finding per gravità e separali dai suggerimenti di stile. Un bug che espone dati non deve perdersi tra rinominazioni facoltative.
FINDING DEBOLE
La gestione errori è sbagliata.
FINDING AZIONABILE
Condizione:
la richiesta API restituisce 401.
Comportamento osservato:
il componente mantiene visibili i dati caricati dall'utente precedente.
Impatto:
possibile esposizione di dati dopo la scadenza della sessione.
Posizione:
[file e riga]
Verifica:
riprodotto forzando una risposta 401 dopo il caricamento iniziale.
Correzione attesa:
svuotare lo stato sensibile e mostrare il flusso di nuova autenticazione. Fonti e metodo di review
GitHub descrive la pull request review come un passaggio per discutere le modifiche, richiedere correzioni e applicare standard di qualità prima del merge. La guida OWASP aggiunge controlli specifici per sicurezza, dati e segreti. Questa checklist combina i due livelli: correttezza rispetto al task e rischio tecnico.
- GitHub Docs: About pull request reviews ↗
- OWASP Code Review Guide ↗
- DottorLone: quando l’approvazione di Copilot può contare per il merge →
- DottorLone: usare VS Code Agent Merge senza delegare il giudizio →
- DottorLone: verificare una patch di GitHub Agentic Autofix →
- DottorLone: proteggere monitoraggio e alert dai refactor agentici →
- DottorLone: provare richiesta, diff e test con Codex su un esercizio isolato →