# `@schema/type any=True` Review

## Candidate for a Subsequent Story

We have a bunch of functions not attached to a type... what does that mean for our overall design?
- if it's intended for use by _a_ type, attach it to that type.
- if it's intended for use by multiple types
- might be a package-scoped function...
- might also want for a grouping type (for namespacing)

processOptionalAnnotation()
-

Is there an issue where checking if an interface type is nil?
- golang interface check nil
- google it ^^^

NewTypeAnnotation()
- All errors should be capitalized
- supports indentation.
- multiple sentences in error message is wordy
- in error messages, use the same words for the same noun

NewMapItemType():
- we’re specifically getting an array value
- something to handle in a constructor? NewArrayType()

setDefaultValues()
- while are we setting the default to nil
- when renaming, review the refactor for semantics.
- could we pull down the “Document” case?

Node.Check() needs to move to Schema

Review panics:
- ensure message describes the violated invariant
- include additional context so long as it doesn't risk causing a subsequent panic.

We’re preventing from making Array Items nullable, but why?
- it adds complexity to the code.

    #@schema/definition
    ---
    foos:
    #@schema/nullable
    - bar: 42
      ree: "things"


    ...
    #@data/values
    ---
    foos: []

    #@data/values
    ---
    foos:
    - #@ { "bar": 1 }
    - #@ null


    ---
    foos:
    - bar: 1
      ree: "things"
    - null



    foos: null

## Nit Picks

newCollectionItemValueType()
- rename to simplify?

TypeAnnotation
- itemPosition — rename? only items?

type.go:
- review spacing between types

---

---
# Schema Overlay Review notes
—
cmd.go:
- why does Values() take a schema? (why not just part of the LibraryLoader?

If we agree that:
for a given `Library` there is exactly one immutable Schema instance covering the Data Values
then:
- accepting schema as a variable to `Values()`, here (erroneously) conveys that this is a variable.
- it would

“DataValue belonging…”
> “DataValue” not a word we use… it’ the document not the Data Value that isn’t used.
- update existing story with two notes:
- the comment above
- that it's the *document* that's not being used

DocumentSchema:

DeepCopyAsNode — don't forget to do this!