lexgen runs all packages together #709
Conversation
cross-package references can now make a type know that it needs to include a type field
There was a problem hiding this comment.
I ran this branch against the atproto repo main branch and the output looks good/expected to me.
Left some notes about flexibility with the lexgen tool with 3rd party lexicons, which hasn't been taken advantage of much to date, but probably will be soon (coming weeks). I can take adding an additional entrypint / sub-command for that use-case when it is needed.
I only skimmed the actual code changes, assuming that @ericvolp12 or @whyrusleeping will review code quality.
| var Packages []Package = []Package{ | ||
| Package{"bsky", "app.bsky", "api/bsky"}, | ||
| Package{"atproto", "com.atproto", "api/atproto"}, | ||
| Package{"chat", "chat.bsky", "api/chat"}, | ||
| Package{"ozone", "tools.ozone", "api/ozone"}, | ||
| } |
There was a problem hiding this comment.
in the near-ish future we are going to want to make it easy for folks to use this tooling for non-bluesky-maintained lexicons (aka, independent lexicons).
in the medium-term, it is most likely that any inter-namespace references would be from independent lexicons to one of these lexicons (aka, independent lexicons referencing com.atproto.* or app.bsky.actor.profile). so having these hard-coded in is fine.
overall I think having these hard-coded is fine for this PR, but I would guess we might want an additional command more like the previous behavior soon (coming weeks).
There was a problem hiding this comment.
I'd replace this (and the go imports package paths) with a json/yaml config input to lexgen
| prefix string // prefix of a major package being processed, e.g. com.atproto | ||
| id string // parent Schema.ID | ||
| defName string // parent Schema.Defs[defName] points to this *TypeSchema |
|
Confirming on my end that the changes to lexicons that I see when running lexgen from this branch look good (just out of date/stale stuff that hasn't been needed on the Go side of things yet). I figure you're gonna merge this and then open another PR with the updated lexicons and requisite changes to make stuff build? Also I do agree with @bnewbold that it may be best to pass the packages into the |
cross-package references can now make a type know that it needs to include a type field