[FUN-747] Add input file option with smaller dependency set #32
Conversation
launchdarkly-upra
bot
changed the title
tvarney13
left a comment
tvarney13
left a comment
There was a problem hiding this comment.
approved but asked a question around potential improvements
| inferedPackage := f.Name.Name | ||
| success := false | ||
| ast.Inspect(f, func(node ast.Node) bool { | ||
| found := writeOptionsFile(typeNames, inferedPackage, node, fset) | ||
| if found { | ||
| success = true | ||
| } | ||
| return !found | ||
| }) | ||
| if !success { | ||
| return fmt.Errorf(`unable to find type "%s"`, typeNames) | ||
| } | ||
|
|
||
| return nil |
There was a problem hiding this comment.
This is semi-duplicated with the thing that main is already doing - should we create a common function? Alternatively can we write a function that constructs the pkgs array differently based on the input provided?
There was a problem hiding this comment.
Looking closer - they're not that similar but there are similar pieces of data with similar types being looked at. There's probably something that could be done, but it's a question of whether or not that effort is worth it for this gen script.
There was a problem hiding this comment.
This is semi-duplicated with the thing that main is already doing - should we create a common function? Alternatively can we write a function that constructs the pkgs array differently based on the input provided?
The main duplication here is the call to ast.Inspect but a lot of the params are sourced differently.
- the method in main loops over all of the files vs single file when using the new mode
- the new method has to infer the package name from the input file
Feature
-input <string>sets the name of the input file. When set uses "go/build" and "go/parser" directly, which can result in performance improvementsrunWithInputFile is an alternative to packages.Load because packages.Load requires a full go driver
runWithInputFile uses "go/build" and "go/parser" directly, but requires a file name to be passed.
This limits the number of required dependencies, and speeds up generation times
Tested against gonfalon with this pr resulting in a no-op: https://github.com/launchdarkly/gonfalon/pull/55614
Related Jira issue: FUN-747: Bazel genrule: go-options