feat: generate ftl-project.toml in the Git root if it DNE #1648
Conversation
deniseli
requested review from
a team and
wesbillman
and removed request for
a team
|
The failing integration test is broken on |
| // CreateDefaultFileIfNonexistent creates the ftl-project.toml file in the Git root if it | ||
| // does not already exist. | ||
| func CreateDefaultFileIfNonexistent(ctx context.Context) error { | ||
| path := GetDefaultConfigPath() |
There was a problem hiding this comment.
Unfortunately GitRoot() can return an empty string, so this can be incorrect. Would you mind converting that function and GetDefaultConfigPath() to use Option[string] so this is more obvious in the future? You'll need to cater to this here as well.
There was a problem hiding this comment.
I'm not sure what we should do if we can't find the git root...
There was a problem hiding this comment.
Ahh, will do. I'll add a warning for that branch case until we come up with something better.
There was a problem hiding this comment.
Done. Let me know what you think of the behavior for all the other GitRoot callers... I defaulted to preserving the old behavior wherever the right choice was ambiguous. At least it's obvious now
There was a problem hiding this comment.
Another consideration is whether to pass through the original error from GitRoot returning (string, error), but I left it as the optional for now.
cmd/ftl/cmd_init.go
Outdated
| gitRoot := internal.GitRoot(dir) | ||
| gitRoot, ok := internal.GitRoot(dir).Get() | ||
| if !ok { | ||
| gitRoot = "" |
There was a problem hiding this comment.
I think this should just return nil no?
| } | ||
|
|
||
| func GetDefaultConfigPath() string { | ||
| return filepath.Join(internal.GitRoot(""), "ftl-project.toml") | ||
| func GetDefaultConfigPath() optional.Option[string] { |
There was a problem hiding this comment.
nit: Idiomatic Go is to not use "getter" style naming, I'd just call this DefaultConfigPath().
| logger := log.FromContext(ctx) | ||
| path, ok := GetDefaultConfigPath().Get() | ||
| if !ok { | ||
| logger.Warnf("Failed to find Git root, so cannot verify whether an ftl-project.toml file exists there") |
Fixes #1639
The current implementation ignores any
--configflag values, but let me know if we should default to creating the last specified config whenever one's provided. For that, I'd just move this code to the resolver and add anr.Configcheck. I decided to minimize it purely to operating on the default path because the specified paths may not actually be at the Git root, which could make the new behavior a bit more complex to comprehend. That said, there could be a good reason to use the flag value that I didn't think of.