Codegen output rearrange #105
Conversation
schema/gen/go/generate.go
Outdated
| // Emit a file with all the Node/NodeBuilder/NodeAssembler boilerplate. | ||
| // Also includes typedefs for representation-level data. | ||
| // Also includes the MaybeT boilerplate. | ||
| withFile(filepath.Join(pth, "ipldsch.satisfaction.gen.go"), func(f io.Writer) { |
There was a problem hiding this comment.
This filename in particular is bit placeholder. Other suggestions welcome.
| // Comments returns a bool for whether comments should be included in gen output or not. | ||
| func (cfg *AdjunctCfg) Comments() bool { | ||
| return true // FUTURE: okay, maybe this should be configurable :) | ||
| } |
There was a problem hiding this comment.
Do we really need this? It's trivial to remove comments from Go source code by just using go/parser with the comment flag disabled, and re-printing the program. Or, if the code is simple enough, a bit of sed.
| // GenerateSplayed emits many more individual files than Generate. | ||
| // | ||
| // This function should be considered deprecated and may be removed in the future. | ||
| func GenerateSplayed(pth string, pkgName string, ts schema.TypeSystem, adjCfg *AdjunctCfg) { |
There was a problem hiding this comment.
I'd personally remove it directly. The old code will always be in the git history if we wish to reuse it.
| for _, tn := range keys { | ||
| switch t2 := types[tn].(type) { | ||
| case *schema.TypeBool: | ||
| fn(NewBoolReprBoolGenerator(pkgName, t2, adjCfg), f) |
There was a problem hiding this comment.
this code feels repetitive to me; it seems like you could do:
var gen func(...)
switch t2 := types[tn].(type) {
case *schema.TypeBool:
gen = NewBoolReprBoolGenerator
case ...
}
fn(gen(pkgName, t2, adjCfg), f)
schema/gen/go/generate.go
Outdated
| } | ||
|
|
||
| // Emit a file with the type table, and the golang type defns for each type. | ||
| withFile(filepath.Join(pth, "ipldsch.types.gen.go"), func(f io.Writer) { |
There was a problem hiding this comment.
not sure I like "ipldsch", it reads like some germanic gibberish to me :)
how about just ipldschema_types.go? it's self-describing, not too long, and honestly I don't think we need the gen.go suffix if we already have an ipldschema_ prefix. underscores before the extension dot are also generally more common.
There was a problem hiding this comment.
I thought (but maybe I imagined) I'd seen a convention ".gen" as a prefix to the file extension suffix as a semi-universal hint that a file is generated. I wish some convention of that existed, anyway. But maybe that was wishful thinking?
There was a problem hiding this comment.
The convention is the "Code generated by..." comment, and nothing else, really. Take stringer, which generates yourtype_string.go.
I think a prefix like ipldschema_ or ipldsch_ is already extremely clear, to be honest. Adding .gen just adds verbosity for a convention that doesn't exist :)
Also, emit some comments around the type definitions. The old file layout is still available, but renamed to GenerateSplayed. It will probably be removed in the future. The new format does not currently have stable output order. I'd like to preserve the original order given by the schema, but our current placeholder types for schema data don't have this. More work needed on this.
I'd still probably prefer to replace this with simply having a stable order that is carried through consistently, but that remains blocked behind getting self-hosted types, and while it so happens I also got about 80% of the way there on those today, the second 80% may take another day. Better make this stable rather than wait.
warpfork
force-pushed
the
codegen-output-rearrange
branch
from
1eaa24c to
4f33395
Compare
An underscore; and less "gen", because reviewers indicated it felt redundant.
|
😱 it looks like this merged a bit too early. I'm seeing the following error on codegen output now: |
cleanup from ipld#105
cleanup from ipld#105
filenames change due to #105 . gofmt also applied for the first time. from here on out: `go generate` should just cause these files to be automagically updated and formatted.
Rearrange codegen output into finite number of files.
Also, emit some comments around the type definitions.
The old file layout is still available, but renamed to GenerateSplayed. It will probably be removed in the future. (Maybe it should be removed now, for that matter.)
All the files produced now match the pattern "
ipldsch.*.gen.go". There's file for the common bits that need to appear in every codegen output package; one file for all the type definitions, including the type/prototype table (I figure this is what a developer will hit most often during quick reference); and one file for everything else (including the MaybeT types, the Node implementations, the NodeAssembler implementations, and all that again for the representation level).This could be a resolution for #93 .