---
# System prepended metadata

title: '`@schema/type any=True` Review'

---


# `@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.


    ```yaml
    #@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!


