Delete StoreRawSchema from GeneratorConfig since it's not being used
#705
Conversation
I don't see the value of why we need this flag: is it to save memory? For `exampleoc` this variable took 27MB, a negligible amount.
|
At this point, I think it is generally not that useful. The original reason that this was a flag is that we didn't create this output by default. (Indeed, removing the flag might cause some tests to fail that don't expect the schema to be there.) 27MB of generated code is quite a lot - it can be the difference between being able to open the code in an editor and not, but I'm OK with removing these flags. We can probably achieve the same by having the schema stored in a separate file or similar. |
Agreed that 27MB of generated code is a lot. However reading this deleted doc comment, this flag was rather supposed to be controlling whether this variable is filled. So by 27MB I actually meant the memory usage of calling |
Hmm I misunderstood the meaning of |
|
Interesting -- I don't believe we ever did this, I think we always compressed the schema and stored it in the generated Go. The integration tests were potentially just taking a shortcut to validating the contents are correct, but I can't see a real use case for this. Can we change the integration test to rather uncompress the gzip and compare this to the expected schema? |
Do you mean compare I propose to leave this be since the raw JSON schema can be used by someone for whatever processing they need during the code generation stage, so it's not completely useless, and we can use it for testing as is currently done. |
|
Ah, I see -- because we don't execute the code, we don't actually get the byte array, we just have the string. Yes, I think in this case it's reasonable to leave it, since I think having an integration test that checks that the JSON generated is correct is more valuable than the limited simplification removing the flag gets us. |
) * ygen now only has `IROptions`. * `CodeGenerator` in gogen/protogen now carry IROptions and their language-specific post-IR generation options. * Code generation specific flags that are not used during IR generation are moved to `gogen` and `protogen`, duplicating if used by both (e.g. CallerName). * `SkipEnumDeduplication` is moved from ParseOptions to TransformationOptions.
* Run go generate ./... for CI check * Regenerate exampleoc but don't depend on it for tests * Delete `StoreRawSchema` from `GeneratorConfig` since it's not being used (#705) * Delete `StoreRawSchema` from `GeneratorConfig` since it's not being used I don't see the value of why we need this flag: is it to save memory? For `exampleoc` this variable took 27MB, a negligible amount. * Reorganize Code Generation flags and Delete `ygen.GeneratorConfig` (#706) * ygen now only has `IROptions`. * `CodeGenerator` in gogen/protogen now carry IROptions and their language-specific post-IR generation options. * Code generation specific flags that are not used during IR generation are moved to `gogen` and `protogen`, duplicating if used by both (e.g. CallerName). * `SkipEnumDeduplication` is moved from ParseOptions to TransformationOptions.
* Fix `IdentityType` classification bug. Right now all typedefs are automatically categorized as `DerivedEnumerationType`; this is wrong because identity types could also be in a typedef. Fixed this bug and added tests. * Remove debugging statement * Add coverage for proto generation of typedef identityref * Run go generate ./... for CI check (#703) * Run go generate ./... for CI check * Regenerate exampleoc but don't depend on it for tests * Delete `StoreRawSchema` from `GeneratorConfig` since it's not being used (#705) * Delete `StoreRawSchema` from `GeneratorConfig` since it's not being used I don't see the value of why we need this flag: is it to save memory? For `exampleoc` this variable took 27MB, a negligible amount. * Reorganize Code Generation flags and Delete `ygen.GeneratorConfig` (#706) * ygen now only has `IROptions`. * `CodeGenerator` in gogen/protogen now carry IROptions and their language-specific post-IR generation options. * Code generation specific flags that are not used during IR generation are moved to `gogen` and `protogen`, duplicating if used by both (e.g. CallerName). * `SkipEnumDeduplication` is moved from ParseOptions to TransformationOptions.
* Delete unused variables and rephrase package comments for ygen. * Fix `IdentityType` classification bug. (#699) * Fix `IdentityType` classification bug. Right now all typedefs are automatically categorized as `DerivedEnumerationType`; this is wrong because identity types could also be in a typedef. Fixed this bug and added tests. * Remove debugging statement * Add coverage for proto generation of typedef identityref * Run go generate ./... for CI check (#703) * Run go generate ./... for CI check * Regenerate exampleoc but don't depend on it for tests * Delete `StoreRawSchema` from `GeneratorConfig` since it's not being used (#705) * Delete `StoreRawSchema` from `GeneratorConfig` since it's not being used I don't see the value of why we need this flag: is it to save memory? For `exampleoc` this variable took 27MB, a negligible amount. * Reorganize Code Generation flags and Delete `ygen.GeneratorConfig` (#706) * ygen now only has `IROptions`. * `CodeGenerator` in gogen/protogen now carry IROptions and their language-specific post-IR generation options. * Code generation specific flags that are not used during IR generation are moved to `gogen` and `protogen`, duplicating if used by both (e.g. CallerName). * `SkipEnumDeduplication` is moved from ParseOptions to TransformationOptions.
* Split Proto generation into `protogen` package. The change includes the following notable changes: * protogen/codegen.go: Instead of using `ygen.NewYANGCodeGenerator` as the struct for all code generation from the single `ygen` package, `protogen.NewProtoCodeGenerator` is created for Proto generation. `ProtoOpts` is moved to `protogen`. * protogen/genir_test: Since IR generation requires a suitable `LangMapper` implementation, the tests using `ProtoLangMapper` are moved to here. * protogen/protogen.go, protoelements.go: These are moved to this package. * integration_tests/generate_errs_test.go: Moved `TestGenerateErrs` out of ygen. * remove unused code * remove unused variable * add coverpkg * Try again with coverpkg * Try again with coverpkg * Fix syntax error * coverpkg=all * coverpkg=./... * coverpkg filter out irrelevant packages * try to fix coverpkg * try to fix coverpkg * use bash instead of coveralls * Remove 'proto' from various exported names * GenerateProto3 -> Generate * Delete unused variables and rephrase package comments for ygen. (#697) * Delete unused variables and rephrase package comments for ygen. * Fix `IdentityType` classification bug. (#699) * Fix `IdentityType` classification bug. Right now all typedefs are automatically categorized as `DerivedEnumerationType`; this is wrong because identity types could also be in a typedef. Fixed this bug and added tests. * Remove debugging statement * Add coverage for proto generation of typedef identityref * Run go generate ./... for CI check (#703) * Run go generate ./... for CI check * Regenerate exampleoc but don't depend on it for tests * Delete `StoreRawSchema` from `GeneratorConfig` since it's not being used (#705) * Delete `StoreRawSchema` from `GeneratorConfig` since it's not being used I don't see the value of why we need this flag: is it to save memory? For `exampleoc` this variable took 27MB, a negligible amount. * Reorganize Code Generation flags and Delete `ygen.GeneratorConfig` (#706) * ygen now only has `IROptions`. * `CodeGenerator` in gogen/protogen now carry IROptions and their language-specific post-IR generation options. * Code generation specific flags that are not used during IR generation are moved to `gogen` and `protogen`, duplicating if used by both (e.g. CallerName). * `SkipEnumDeduplication` is moved from ParseOptions to TransformationOptions.
* Make `EnumeratedYANGType.IdentityBaseName` exported. I think it makes sense for this to be exported. Not sure why I wanted it to be extracted from the plugin originally. * Add `LangMapperExt` and `Flags` fields to each level of the `IR`. The idea is to to call a different method from the interface for each level of hierarchy of the IR to populate the `Flag` fields for that level of the IR. So within `getOrderedDirDetails` for each `ParsedDirectory` and `NodeDetails`, and within the loop of genEnums (in `GenerateIR`) for each `EnumeratedYANGType`. This way the caller of `GenerateIR` is able to produce extra information for any level of the `IR`. Tests added in genir_test.go * Generate `YANGType` using `LangMapperExt` mechanism. * Remove dependency of enumgen on MappedType * Move enumset and schematree into `LangMapperBase`. Currently, the individual LangMapper implementation instances (GoLangMapper and ProtoLangMapper) are required to hold these types, which can be confusing to the user since these are not fundamentally part of the LangMapper API -- rather it's their methods (i.e. enum name look-up and leafref resolution) that are part of the LangMapper API interface. Embedding a special interface (`LangMapperBase`) not only decouples the implementation from the interface, but also provides a clear forward compatibility contract of methods (incl. enum name look-up and leafref resolution) available to implementors should more functionality become available in the future. The proposed vision is: * `LangMapper`-defined methods need to be implemented by LangMapper implementors. * `LangMapperBaseSetup`-defined methods are set-up methods that are called either within `GenerateIR`, or by the unit tests of LangMapper methods. * `LangMapperBase`-defined methods NOT part of the overall `LangMapper` interface are helper methods available to `LangMapper` implementors in their method implementations. I will modify the design doc once all is agreed-upon and merged. * Add `SetupEnumSet` and `SetupSchemaTree` to `LangMapperBase`. This allows unit tests to be created by custom LangMappers on their methods (e.g. `FieldName`, `KeyLeafType`). * Delete `YANGCodeGenerator.GetDirectoriesAndLeafTypes` * Split Go generation into `gogen` package. The change includes the following notable changes: * gogen/codegen.go: Instead of using `ygen.NewYANGCodeGenerator` as the struct for all code generation from the single `ygen` package, `gogen.NewGoCodeGenerator` is created for Go generation. `GoOpts` is moved in `gogen`. * gogen/genir_test: Since IR generation requires a suitable `LangMapper` implementation, the tests using `GoLangMapper` are moved to here. * gogen/gogen.go, goelements.go: These are moved to this package. * gogen/helpers.go: Go generation-specific helpers are moved here. * internal/igenutil/genutil.go: Non-Go-specific generation helpers are moved here. * ygen/genstate_test.go: A fake LangMapper implementation is created for unit testing. * Fix Go workflow * remove unused code * Fix workflow * Use bash instead of overalls * Overwrite report for gogen * Improve comments * Change enumeratedTypedefTypeName to return a bool for whether the input type is actually a typedef * Rename Setup to Inject and fix-up comments * Don't install overalls * Remove Go from some some public types/functions in gogen * Split Proto generation into `protogen` package. (#696) * Split Proto generation into `protogen` package. The change includes the following notable changes: * protogen/codegen.go: Instead of using `ygen.NewYANGCodeGenerator` as the struct for all code generation from the single `ygen` package, `protogen.NewProtoCodeGenerator` is created for Proto generation. `ProtoOpts` is moved to `protogen`. * protogen/genir_test: Since IR generation requires a suitable `LangMapper` implementation, the tests using `ProtoLangMapper` are moved to here. * protogen/protogen.go, protoelements.go: These are moved to this package. * integration_tests/generate_errs_test.go: Moved `TestGenerateErrs` out of ygen. * remove unused code * remove unused variable * add coverpkg * Try again with coverpkg * Try again with coverpkg * Fix syntax error * coverpkg=all * coverpkg=./... * coverpkg filter out irrelevant packages * try to fix coverpkg * try to fix coverpkg * use bash instead of coveralls * Remove 'proto' from various exported names * GenerateProto3 -> Generate * Delete unused variables and rephrase package comments for ygen. (#697) * Delete unused variables and rephrase package comments for ygen. * Fix `IdentityType` classification bug. (#699) * Fix `IdentityType` classification bug. Right now all typedefs are automatically categorized as `DerivedEnumerationType`; this is wrong because identity types could also be in a typedef. Fixed this bug and added tests. * Remove debugging statement * Add coverage for proto generation of typedef identityref * Run go generate ./... for CI check (#703) * Run go generate ./... for CI check * Regenerate exampleoc but don't depend on it for tests * Delete `StoreRawSchema` from `GeneratorConfig` since it's not being used (#705) * Delete `StoreRawSchema` from `GeneratorConfig` since it's not being used I don't see the value of why we need this flag: is it to save memory? For `exampleoc` this variable took 27MB, a negligible amount. * Reorganize Code Generation flags and Delete `ygen.GeneratorConfig` (#706) * ygen now only has `IROptions`. * `CodeGenerator` in gogen/protogen now carry IROptions and their language-specific post-IR generation options. * Code generation specific flags that are not used during IR generation are moved to `gogen` and `protogen`, duplicating if used by both (e.g. CallerName). * `SkipEnumDeduplication` is moved from ParseOptions to TransformationOptions.
I don't see the value of why we need this flag: is it to save memory?
For
exampleocRawJSONSchematook 27MB, a negligible amount.