-
Notifications
You must be signed in to change notification settings - Fork 26
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
init: Option to write config file to stdout. #47
Conversation
Codecov Report
@@ Coverage Diff @@
## master #47 +/- ##
=======================================
Coverage 66.23% 66.23%
=======================================
Files 33 33
Lines 1869 1869
=======================================
Hits 1238 1238
Misses 566 566
Partials 65 65 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good to me and works on my local machine👍
Minor question below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. minor suggestions.
fmt.Printf("\nOnce the config file is updated, start Conduit with:\n") | ||
fmt.Printf(" ./conduit -d %s\n", path) | ||
// If a data dir is created, print some additional help. | ||
if path != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: it may be better to use configWriter==nil
check here; one fewer var to track. path
is getting updated at line 58 and it only happens when configWriter == nil
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
configWriter is set to the file on line 74. I agree that the logic is strange, maybe I should just refactor to avoid the strange logic.
Co-authored-by: algochoi <86622919+algochoi@users.noreply.github.com>
Summary
While creating a docker container, I found that it could be used like
docker run algorand/conduit init
to create a config file. The problem is that output is written to a file.This changes the behavior of
conduit init
. Previously it would create a directory nameddata
containing the config file, now it writes the config file to stdout. To get the previous behavior you would use the commandconduit init -d data
.Test Plan
Unit tests are updated to cover the new behavior.