Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

Dynamic Review: an incremental review with an ancestor as previous commit becomes a full review

Aperta
  • Qualcosa non va?
#306 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
35/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
git, typescript
Ambito
backend

Direzione di ricerca

Start in the handler that catches ObjectUnavailable from tracked_tree(previous) and sets previous = None, then follow the previous_reviewed_sha not in store.objects check that skips the fetch after the filter tree:0 ancestry walk. Reproduce with the history base -> c1 -> c2 and an incremental claim, using the integration tests that run against real upload-pack. Done means the ancestor case reports comparison == "incremental" with the right previous_diff, while the refused-server, unknown-commit and sibling cases keep their current behaviour.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

area: runner type: bug

Found while reviewing PR #300 (review finding F4). Pre-existing: it is not caused by #296.

Problem

An incremental review whose previous reviewed commit is an ancestor of the head, the usual case when new commits are pushed on top, silently becomes a full review. The commits-only ancestry walk (filter tree:0) already stores the previous commit, so if previous_reviewed_sha not in store.objects skips the fetch. tracked_tree(previous) then finds no tree, raises ObjectUnavailable, and the handler turns that into previous = None: comparison is full and previous_diff is empty.

Reproduced on the code before and after #296 with the history base -> c1 -> c2, previous = c1, base branch = base, claim incremental. The existing incremental tests only use a previous commit that is not an ancestor, so none of them covers this.

Design

  • Decide whether the previous commit's tree is present, not whether the commit is: fetch the previous commit when its tree is absent (a depth-1 fetch with the size filter), then read the tree.
  • Keep the existing fallback (previous commit explicitly refused by the server → full mode with the prior state kept).
  • Add a test with a linear history.

Test cases

ID kind level case expected
T01 regression integration (real upload-pack) previous = ancestor of head comparison == "incremental", previous_diff holds the change since the previous commit; red before the fix
T02 regression integration previous = sibling of head unchanged: incremental
T03 behavior integration previous commit refused by the server full mode, prior state kept
T04 behavior integration previous commit unknown to the repository full mode
T05 behavior unit (checkout) the tree of the previous commit is absent but the commit is present the fetch happens once
Lingua principale
TypeScript
Stelle
0
Fork
1
Merge medio
1h 22m
PR unite (30g)
336

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di ExpidetGames/code-reviewer

Tutte le issue di ExpidetGames/code-reviewer

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.