Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Temp provide autoconf defaults #650

Merged
merged 6 commits into from
Aug 15, 2024
Merged

Temp provide autoconf defaults #650

merged 6 commits into from
Aug 15, 2024

Conversation

sourishkrout
Copy link
Member

@sourishkrout sourishkrout commented Aug 8, 2024

TBD if we automatically use defaults transparently or prompt the user to write a runme.yaml to disk. However, for the time being this does not hard block runme beta commands without much explanation about the absence of runme.yaml.

If we stick with transparent defaults we should probably rename RootConfig to RunmeConfig or some such.

Changes to the config protos were necessary to distinguish between null bools versus false.

@sourishkrout sourishkrout requested a review from adambabik August 8, 2024 23:44
@sourishkrout
Copy link
Member Author

I'm now realizing I could have done this with dependency injection 🤔

https://github.com/stateful/runme/blob/seb/autoconf-defaults/internal/runnerv2service/service_execute_test.go#L35-L45

@sourishkrout sourishkrout force-pushed the seb/autoconf-defaults branch from 452e0e5 to 804207f Compare August 12, 2024 20:56
internal/config/autoconfig/autoconfig.go Outdated Show resolved Hide resolved
internal/config/config.go Outdated Show resolved Hide resolved
internal/config/config_defaults.go Outdated Show resolved Hide resolved
internal/tls/tls.go Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Aug 15, 2024

@sourishkrout sourishkrout merged commit c186a13 into main Aug 15, 2024
7 checks passed
@sourishkrout sourishkrout deleted the seb/autoconf-defaults branch August 15, 2024 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants