review

review patchsets using your default editor
Log | Files | Refs

duplication.md (2921B)


      1 # Duplication
      2 
      3 You are given each name the change adds, and a list of existing declarations found by
      4 searching for its words. Decide whether the new thing already exists.
      5 
      6 - `already-named` — the repository already has a name for this concept. A second
      7   name for one idea is worse than an awkward first one.
      8 - `restates-a-fact` — a constant or literal states something already stated
      9   elsewhere: the same magic bytes, the same size, the same layout, written twice.
     10   Facts belong to one owner.
     11 - `in-the-stdlib` — the standard library already defines this. A hand-copied
     12   constant (`0x2000` for `IMAGE_FILE_DLL`) is the common case.
     13 - `layout-crosses-a-seam` — one package states the byte layout, field order or
     14   wire format that another package owns. A size constant describing a structure
     15   another package reads or writes is the usual shape of it. The owner should
     16   expose it.
     17 
     18 An error value is the exception to all of these. A package declares its own
     19 sentinels so that its callers can test for them without importing somewhere else,
     20 and that is part of its interface however identical the text is. Never report one
     21 error value as a duplicate of another.
     22 
     23 ## The pass to make
     24 
     25 Before judging anything, list the declarations that have a candidate marked
     26 `<- same value`. Then judge every entry on that list, including the ones that come
     27 after the first duplicate you find. Stopping early because a duplicate has already
     28 been reported is the mistake this job makes most often.
     29 
     30 A declaration with several same-value candidates is judged against each of them in
     31 turn, and each pair that survives is its own finding.
     32 
     33 For each pair, say what the two values measure, then compare the answers:
     34 
     35 - Both measure the same field of the same structure — the same header, the same
     36   directory row, the same signature, the same magic. One fact with two owners:
     37   report it, and say which declaration should own it.
     38 - One identifies and the other measures — a resource type id beside a row width, a
     39   count beside a byte size, an index beside a length. Different facts that happen to
     40   be equal: say nothing about them.
     41 
     42 A header size and another package's directory size that are equal because they
     43 describe the same header are the case this job exists for. Report each such pair
     44 separately, one finding each.
     45 
     46 Only report when the candidate list actually contains the thing. You cannot see the
     47 whole repository, so do not guess that something exists somewhere. If the candidates
     48 do not show a duplicate, report nothing.
     49 
     50 ## Reading across languages
     51 
     52 The repository's declarations arrive read by the language's own grammar, so a
     53 name is found however it is laid out and a constant's value is compared
     54 whether the line says `=` or `::`. The error-value exception above is a Go and
     55 TypeScript shape: a package owns its sentinels. A language that reports errors
     56 by returning them has nothing of that shape to skip.