-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add monogen tool #213
Add monogen tool #213
Conversation
WalkthroughThe pull request introduces several modifications across multiple files, including the addition of a Changes
Possibly related PRs
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (3)
Files skipped from review as they are similar to previous changes (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
9060562
to
aa754a9
Compare
aa754a9
to
70099f7
Compare
To try it out yourself, run make monogen \
&& cd .. \
&& ./monomer/bin/monogen \
&& cd testapp \
&& ../monomer/monogen/monogen.sh \
&& ./testappd monomer start --minimum-gas-prices 0.01stake Two follow-ups are planned:
The idea is that we can instruct users to run go run github.com/polymerdao/monomer/cmd/monogen@latest and automatically paste |
I'll fix the lints tomorrow. This is ready for review otherwise. This is a monolithic change; it will not be useful to review commit-by-commit. |
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.
Actionable comments posted: 1
Outside diff range and nitpick comments (3)
monogen/monogen.sh (1)
1-40
: Great work on automating the initialization and setup process for a Cosmos SDK-based application!The script is well-structured, follows best practices, and provides useful comments and documentation. It automates the initialization and setup process for a Cosmos SDK-based application, which is a good practice and will save time and reduce errors.
As a suggestion for improvement, consider adding a help message or usage instructions at the beginning of the script to provide more information about what the script does and how to use it. This will make it easier for other developers to understand and use the script.
monogen/monogen.go (2)
19-20
: Address the TODO comment.The TODO comment indicates that the function should handle the case when the directory already exists. Please ensure that this case is handled appropriately to avoid overwriting existing files or throwing an error.
Do you want me to generate the code to handle this case or open a GitHub issue to track this task?
18-89
: Consider decomposing the function into smaller functions.The
Generate
function is quite large and performs several distinct tasks. Consider decomposing it into smaller functions for better readability and maintainability. For example, you could extract the code for applying modifications into a separate function.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
Files selected for processing (7)
- .gitignore (1 hunks)
- Makefile (2 hunks)
- go.mod (12 hunks)
- monogen/cmd/main.go (1 hunks)
- monogen/monogen.go (1 hunks)
- monogen/monogen.sh (1 hunks)
- monogen/monogen_test.go (1 hunks)
Files skipped from review due to trivial changes (1)
- .gitignore
Additional context used
GitHub Check: go
monogen/cmd/main.go
[failure] 43-43:
exitAfterDefer: os.Exit will exit, anddefer cancel()
will not run (gocritic)
Additional comments not posted (33)
monogen/monogen.sh (4)
1-4
: LGTM!The shebang line is correct, and the
set -e
command is a good practice to stop the script execution if any command fails.
6-10
: LGTM!The code segment correctly checks the Go version and sets the
ldflags
variable accordingly. Thego build
command is used to build the binary with the correct flags.
12-34
: LGTM!The code segment correctly initializes the application's config and data directories, creates a dummy account, funds it at genesis, and facilitates the creation of a genesis transaction for the dummy account to self-delegate as a validator. Finally, it collects the generated transactions into the genesis file.
36-40
: LGTM!The comment is informative and provides a useful command to start the application with configurable minimum gas prices.
monogen/monogen_test.go (3)
16-62
: Comprehensive and well-structured test formonogen.Generate
.The test function
TestGenerate
is well-designed and covers the essential aspects of themonogen.Generate
function. It follows a clear and logical flow, setting up the environment, executing the generation script, starting the application, and performing a connectivity check with the blockchain.The test takes appropriate measures to ensure that it does not interfere with the user's environment by using a temporary directory and custom home environment variable. It effectively validates the integration of the
monogen
functionality with the CometBFT client by verifying the connectivity and retrieving the block information.The test uses appropriate assertions to verify the expected behavior and handles the cleanup of the started application using
defer
statements. The use of a customlogWriter
to capture the output of the executed commands is a nice touch for debugging purposes.Overall, this test provides confidence in the correctness of the
monogen.Generate
function and its integration with the CometBFT client.
64-64
: Clear and concise type alias for logging functionality.The
logWriter
type alias provides a clear and concise way to define the logging functionality used in the test function. By using a function type that takes a variadic parameter of typeany
, it can be easily used with thet.Log
function to capture the output of the executed commands.The type alias improves the readability of the code and encapsulates the logging functionality in a meaningful way.
66-69
: Effective implementation of theio.Writer
interface for logging.The
Write
method on thelogWriter
type correctly implements theio.Writer
interface, allowing it to be used for capturing the output of the executed commands in the test function.By logging the string representation of the input byte slice using the
logWriter
function, it effectively captures the output. The method adheres to theio.Writer
interface contract by returning the length of the input byte slice and anil
error.This implementation enables seamless integration with the
cmd.Stdout
andcmd.Stderr
fields, facilitating the capture of command output for debugging purposes.Makefile (4)
5-5
: LGTM!The
BIN
variable is defined correctly using the conditional assignment operator, allowing the user to override the default value if needed. The variable is used appropriately in themonogen
target to specify the output directory for the binary.
13-13
: LGTM!The
monogen
target is declared correctly as a phony target using the.PHONY
directive. This ensures that the target is always executed, regardless of the existence of a file namedmonogen
in the directory.
14-15
: LGTM!The
monogen
target is defined correctly to compile the Go program located in./monogen/cmd
using thego build
command. The-o
flag is used appropriately to specify the output directory and binary name using theBIN
variable.
94-94
: LGTM!The
clean
target is updated correctly to remove the$(BIN)
directory if it exists. Theif
statement is used appropriately to check for the directory's existence before attempting to remove it recursively using therm -r
command. This ensures effective cleanup of the build artifacts.monogen/monogen.go (4)
91-158
: LGTM!The
addRollupModule
function correctly adds the rollup module to the application. The changes are well-structured and follow best practices.
160-211
: LGTM!The
removeConsensusModule
function correctly removes the consensus module from the application. The changes are well-structured and follow best practices.
213-242
: LGTM!The
addMonomerCommand
function correctly adds the Monomer command to the application's command structure. The changes are well-structured and follow best practices.
244-263
: LGTM!The
addReplaceDirectives
function correctly adds the replace directives to the go.mod file. The changes are well-structured and follow best practices.go.mod (18)
4-4
: LGTM!The minor version update for
cosmossdk.io/api
is unlikely to introduce breaking changes and may include bug fixes or minor enhancements.
12-12
: LGTM!The patch version update for
github.com/cometbft/cometbft
is unlikely to introduce breaking changes and may include bug fixes or performance improvements.
16-16
: LGTM!The patch version update for
github.com/cosmos/cosmos-sdk
is unlikely to introduce breaking changes and may include bug fixes or performance improvements.
24-24
: LGTM!The patch version update for
github.com/gorilla/mux
is unlikely to introduce breaking changes and may include bug fixes or performance improvements.
32-32
: LGTM!The patch version update for
github.com/spf13/cobra
is unlikely to introduce breaking changes and may include bug fixes or performance improvements.
35-35
: LGTM!The version update for
golang.org/x/exp
to a newer commit hash is unlikely to introduce breaking changes and may include bug fixes, performance improvements, or new experimental features.
43-43
: LGTM!The patch version update for
cosmossdk.io/x/tx
is unlikely to introduce breaking changes and may include bug fixes or performance improvements.
44-44
: LGTM!The addition of the indirect dependency
dario.cat/mergo
is likely required by one of the direct dependencies and should not cause any issues.
47-47
: LGTM!The patch version update for
github.com/99designs/keyring
is unlikely to introduce breaking changes and may include bug fixes or performance improvements.
74-74
: LGTM!The minor version update for
github.com/cenkalti/backoff/v4
is unlikely to introduce breaking changes and may include new features or enhancements.
94-94
: LGTM!The patch version update for
github.com/cpuguy83/go-md2man/v2
is unlikely to introduce breaking changes and may include bug fixes or performance improvements.
98-98
: LGTM!The minor version update for
github.com/danieljoos/wincred
is unlikely to introduce breaking changes and may include new features or enhancements.
121-121
: LGTM!The minor version update for
github.com/fatih/color
is unlikely to introduce breaking changes and may include new features or enhancements.
188-188
: LGTM!The patch version update for
github.com/hashicorp/go-hclog
is unlikely to introduce breaking changes and may include bug fixes or performance improvements.
193-193
: LGTM!The minor version update for
github.com/hashicorp/go-plugin
is unlikely to introduce breaking changes and may include new features or enhancements.
196-196
: LGTM!The patch version update for
github.com/hashicorp/golang-lru/v2
is unlikely to introduce breaking changes and may include bug fixes or performance improvements.
294-294
: LGTM!The minor version update for
github.com/pelletier/go-toml/v2
is unlikely to introduce breaking changes and may include new features or enhancements.
352-352
: LGTM!The patch version update for
go.etcd.io/bbolt
is unlikely to introduce breaking changes and may include bug fixes or performance improvements.
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! Left a few minor nits and questions but the impl looks solid
Summary by CodeRabbit
New Features
monogen
for scaffolding Cosmos SDK projects.monogen.sh
for initializing Cosmos SDK applications.Bug Fixes
Chores
.gitignore
to manage ignored files more effectively.