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
Launchpad: simulation: close open files, fix error checks #8226
Conversation
From: #8217 Closes: #6784 Thanks: @PrathyushaLakkireddy for the original patch. Co-authored-by: Alessio Treglia <alessio@tendermint.com> Co-authored-by: Anil Kumar Kammari <anil@vitwit.com>
|
||
folderPath := os.ExpandEnv("$HOME/.simapp/simulations") | ||
fileName := fmt.Sprintf("%s.log", time.Now().Format("2006-01-02_15:04:05")) | ||
folderPath := path.Join(os.ExpandEnv("$HOME"), ".simapp", "simulations") |
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.
why is this always simapp?
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.
This is used only for simulations, so it lgtm
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.
yeah but more apps rely on this "module", including gaia, so it would check for the .simapp directory
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.
Let's just use simulation
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.
This needs to be fixed in master first.
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
This requires approval from @fedekunze and @clevinson |
Pinging @fedekunze & @clevinson for approvals |
From: #8217
Closes: #6784
Thanks: @PrathyushaLakkireddy for the original patch.
Co-authored-by: Alessio Treglia alessio@tendermint.com
Co-authored-by: Anil Kumar Kammari anil@vitwit.com
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passes