-
Notifications
You must be signed in to change notification settings - Fork 57
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
Yarik/gm world frontend #372
Conversation
WalkthroughThe updates bring enhancements and simplifications across various scripts and tutorials. Key changes include version updates for Node.js and Yarn, restructured tutorial content for better clarity, and modifications in shell scripts to improve installation processes and environment setup. These changes aim to streamline development setups and improve user guidance in connecting wallets to blockchain applications. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
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
Out of diff range and nitpick comments (6)
tutorials/gm-world-frontend.md (3)
12-12
: Ensure fenced code blocks are surrounded by blank lines for proper formatting.
15-15
: Ensure fenced code blocks are surrounded by blank lines for proper formatting.
Line range hint
49-49
: Remove multiple consecutive blank lines to maintain clean and professional documentation.- -public/install-go.sh (1)
Line range hint
31-31
: Ensure variables are enclosed in double quotes to prevent globbing and word splitting, enhancing the script's robustness and security.- current="$(/usr/local/go/bin/go version 2>/dev/null | awk '{print $3}')" + current="$("/usr/local/go/bin/go version" 2>/dev/null | awk '{print $3}')"tutorials/gm-world.md (2)
40-40
: Avoid using inline HTML where possible to keep markdown clean and maintainable. Consider using markdown syntax or Vue components if supported.
Line range hint
42-42
: Remove multiple consecutive blank lines to maintain clean and professional documentation.- -
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- public/install-gm-frontend-app.sh (1 hunks)
- public/install-go.sh (1 hunks)
- tutorials/gm-world-frontend.md (1 hunks)
- tutorials/gm-world.md (1 hunks)
Files skipped from review due to trivial changes (1)
- public/install-gm-frontend-app.sh
Additional Context Used
ShellCheck (4)
public/install-go.sh (4)
[info] 31-31: Double quote to prevent globbing and word splitting.
[info] 31-31: Double quote to prevent globbing and word splitting.
[info] 31-31: Double quote to prevent globbing and word splitting.
[warning] 114-114: ShellCheck can't follow non-constant source. Use a directive to specify location.
Additional comments not posted (1)
public/install-go.sh (1)
120-120
: Using$(which go)
is a good improvement for flexibility in finding the Go binary. This change should make the script more robust across different environments.
* Add celestia-da tutorial, network agnostic * Update vitepress config * Add link to /guides/gas-price * Update tutorials/celestia-da.md Co-authored-by: joshcs.eth <[email protected]> * Update tutorials/celestia-da.md Co-authored-by: joshcs.eth <[email protected]> * Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update tutorials/celestia-da.md * Update tutorials/celestia-da.md * Update tutorials/celestia-da.md * Apply suggestions from code review * Apply suggestions from code review * Update tutorials/gm-world.md Co-authored-by: joshcs.eth <[email protected]> * Remove sudo, we install gmd, no need to use sudo now * docs: use 1 line for install instead of 2 --------- Co-authored-by: joshcs.eth <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
40d34f6
to
296af03
Compare
296af03
to
11ceb7b
Compare
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: 9
Out of diff range and nitpick comments (1)
tutorials/gm-world-frontend.md (1)
Line range hint
55-55
: Remove multiple consecutive blank lines to adhere to markdown best practices.- -
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- .vitepress/constants/constants.js (1 hunks)
- public/install-yarn.sh (1 hunks)
- tutorials/gm-world-frontend.md (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .vitepress/constants/constants.js
Additional Context Used
LanguageTool (1)
tutorials/gm-world-frontend.md (1)
Near line 17: The official spelling of this programming framework is “Node.js”.
Context: ...ment. :::tip If you don't have yarn or nodejs, run this command to install it using c...
ShellCheck (8)
public/install-yarn.sh (8)
[warning] 33-33: In POSIX sh, 'source' in place of '.' is undefined.
[warning] 33-33: ShellCheck can't follow non-constant source. Use a directive to specify location.
[info] 36-36: Double quote to prevent globbing and word splitting.
[info] 39-39: Double quote to prevent globbing and word splitting.
[info] 44-44: Double quote to prevent globbing and word splitting.
[info] 48-48: Double quote to prevent globbing and word splitting.
[info] 48-48: Double quote to prevent globbing and word splitting.
[info] 48-48: Double quote to prevent globbing and word splitting.
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
Out of diff range and nitpick comments (1)
public/install-jq.sh (1)
Line range hint
3-40
: Ensure all conditional blocks are properly closed with 'fi'.The static analysis hints at potential issues with missing 'fi' statements for 'if' blocks. This can cause the script to fail during execution. Please ensure that all 'if' statements are properly closed with 'fi'. Here's a potential fix:
+ fi # Add this at the end of the macOS installation block
This addition ensures that the script maintains proper structure and avoids runtime errors.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- public/install-jq.sh (1 hunks)
Additional Context Used
ShellCheck (4)
public/install-jq.sh (4)
[error] 3-3: Couldn't find 'fi' for this 'if'.
[error] 3-3: Couldn't parse this if expression. Fix to allow more checks.
[error] 40-40: Expected 'fi' matching previously mentioned 'if'.
[error] 40-40: Expected 'fi'. Fix any mentioned problems and try again.
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: 4
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- public/install-go.sh (3 hunks)
- public/install-jq.sh (1 hunks)
Additional Context Used
ShellCheck (9)
public/install-go.sh (3)
[style] 32-32: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
[style] 34-34: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.
[warning] 124-124: ShellCheck can't follow non-constant source. Use a directive to specify location.
public/install-jq.sh (6)
[info] 10-10: Expressions don't expand in single quotes, use double quotes for that.
[info] 11-11: Not following: ./.bash_profile was not specified as input (see shellcheck -x).
[info] 13-13: Expressions don't expand in single quotes, use double quotes for that.
[info] 14-14: Not following: ./.bashrc was not specified as input (see shellcheck -x).
[info] 16-16: Expressions don't expand in single quotes, use double quotes for that.
[info] 17-17: Not following: ./.zshrc was not specified as input (see shellcheck -x).
Additional comments not posted (4)
public/install-jq.sh (3)
9-21
: Consider simplifying the PATH update logic for macOS.This modification checks for multiple shell configuration files and updates the first one it finds, reducing redundancy and improving maintainability.
26-37
: Linux installation block looks good.
39-41
: Error handling for unsupported operating systems is appropriate.public/install-go.sh (1)
Line range hint
124-124
: Ensure that the source path is constant to avoid issues with dynamic sourcing.
[WARNING]
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- public/install-jq.sh (1 hunks)
Additional Context Used
ShellCheck (3)
public/install-jq.sh (3)
[info] 11-11: Not following: ./.bash_profile was not specified as input (see shellcheck -x).
[info] 14-14: Not following: ./.bashrc was not specified as input (see shellcheck -x).
[info] 17-17: Not following: ./.zshrc was not specified as input (see shellcheck -x).
Additional comments not posted (4)
public/install-jq.sh (4)
10-10
: The use of double quotes for PATH updates is correct as it allows variable expansion. Good job on this update!Also applies to: 13-13, 16-16
26-37
: The handling of different package managers (apt
andyum
) for Linux installations is robust and well-implemented.
19-20
: The error handling for unsupported shells and operating systems is clear and effective, correctly preventing further execution of the script.Also applies to: 34-35, 39-40
4-4
: The installation messages provide clear feedback to the user about the actions being performed by the script. This enhances user interaction and debugging.Also applies to: 24-24, 26-26, 37-37, 39-39
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
branch needs to be updated, still has the celestia-da stuff which has been merged right?
Yeah, merged main |
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: 6
Out of diff range and nitpick comments (1)
tutorials/gm-world-frontend.md (1)
Line range hint
57-57
: Remove extra blank lines to maintain clean and consistent formatting.- -
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
public/img/gm-world-frontend-wallet-connected.png
is excluded by!**/*.png
Files selected for processing (2)
- public/install-go.sh (3 hunks)
- tutorials/gm-world-frontend.md (1 hunks)
Additional Context Used
LanguageTool (1)
tutorials/gm-world-frontend.md (1)
Near line 12: Possible missing comma found.
Context: ...orld) tutorial. Your rollup needs to be running since the app will connect to it via RP...
ShellCheck (3)
public/install-go.sh (3)
[style] 35-35: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
[style] 37-37: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.
[warning] 127-127: ShellCheck can't follow non-constant source. Use a directive to specify location.
Additional comments not posted (2)
tutorials/gm-world-frontend.md (1)
6-6
: Update the title to reflect the new focus of the tutorial.public/install-go.sh (1)
8-11
: Add error handling for missingcurl
to prevent script execution failure.
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: 6
Out of diff range and nitpick comments (7)
tutorials/gm-world.md (7)
Line range hint
1-1
: Correct the title for consistency and professionalism.- # GM world rollup + # GM World Rollup
Line range hint
60-60
: Add a comma for better readability.- To make it simple we will download a repository with a `gm-world` rollup that has an `init.sh` script that does all the setup for you. + To make it simple, we will download a repository with a `gm-world` rollup that has an `init.sh` script that does all the setup for you.
Line range hint
64-64
: Consider a shorter alternative to avoid wordiness.- In order to run it you need to have the jq command line tool installed. You can install it by running `sudo apt-get install jq` on Ubuntu or `brew install jq` on macOS. + To run it, you need the jq command line tool. Install it with `sudo apt-get install jq` on Ubuntu or `brew install jq` on macOS.
Line range hint
78-78
: Ensure fenced code blocks are surrounded by blank lines for proper markdown formatting.+ gmd start --rollkit.aggregator --minimum-gas-prices="0.025stake" --rollkit.da_address http://localhost:7980 +
Line range hint
140-140
: Add a comma for better readability.- For convenience we export two of our keys like this: + For convenience, we export two of our keys like this:
Line range hint
147-147
: If this is a compound adjective that modifies the following noun, use a hyphen.- just want to submit transaction from a high level perspective): + just want to submit transaction from a high-level perspective):
Line range hint
160-160
: Ensure fenced code blocks are surrounded by blank lines for proper markdown formatting.+ gmd query bank balances $KEY2 +
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- tutorials/gm-world-frontend.md (1 hunks)
- tutorials/gm-world.md (2 hunks)
Additional Context Used
LanguageTool (5)
tutorials/gm-world-frontend.md (1)
Near line 17: Possible missing comma found.
Context: ...orld) tutorial. Your rollup needs to be running since the app will connect to it via RP...tutorials/gm-world.md (4)
Near line 60: Possible missing comma found.
Context: ...are your rollup blockchain. To make it simple we will download a repository with a `g...
Near line 64: Consider a shorter alternative to avoid wordiness.
Context: ... script in a new terminal: ::: warning In order to run it you need to have the jq command ...
Near line 140: A comma is probably missing here.
Context: ...1lv8c1FWhLpqP"}' type: local ``` For convenience we export two of our keys like this: `...
Near line 147: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... just want to submit transaction from a high level perspective): ```bash gmd tx bank send...
Additional comments not posted (5)
tutorials/gm-world-frontend.md (4)
22-22
: Correct the spelling of "Node.js" for consistency and professionalism.- If you don't have Yarn or Node.js, run this command to install it using cURL on most Linux distros and macOS: + If you don't have Yarn or Node.js, run this command to install it using cURL on most Linux distros and macOS:
24-24
: Specify the language for fenced code blocks to enable syntax highlighting.- ```bash-vue + ```bash
25-25
: Ensure fenced code blocks are surrounded by blank lines for proper markdown formatting.+ curl -sSL https://rollkit.dev/install-yarn.sh | bash -s {{constants.nodeVersion}} {{constants.yarnVersion}} +
35-35
: Ensure fenced code blocks are surrounded by blank lines for proper markdown formatting.+ curl -sSL https://rollkit.dev/install-gm-frontend-app.sh | bash +tutorials/gm-world.md (1)
37-37
: Specify the language for fenced code blocks to enable syntax highlighting.- ```bash-vue + ```bash
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: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- tutorials/gm-world-frontend.md (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- tutorials/gm-world-frontend.md
@@ -34,7 +34,7 @@ Rollkit uses the [Go programming language](https://go.dev/dl/). Here's how to in | |||
- **Linux or macOS**: Run the provided script: | |||
|
|||
```bash-vue | |||
curl -sSL https://rollkit.dev/install-go.sh | sh -s {{constants.golangVersion}} | |||
curl -sSL https://rollkit.dev/install-go.sh | bash -s {{constants.golangVersion}} |
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.
can we use the same format as bash -c "$(curl -sSL https://rollkit.dev/install-gm-rollup.sh)"
so that the script isn't saved locally, and also pass in the golang version/local da version?
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 can be done in a new PR
@@ -1,6 +1,9 @@ | |||
const constants = Object.freeze({ | |||
golangVersion: "1.22.2", | |||
|
|||
nodeVersion: "21.7.2", | |||
yarnVersion: "1.22.19", |
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.
(comment 1/2) note to remember to update this and also the script when versions change
|
||
set -e | ||
|
||
INSTALL_NODE_VER=21.7.2 |
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.
(comment 2/2) Q: how should we maintain these along with the variables?
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: 2
Out of diff range and nitpick comments (1)
public/install-go.sh (1)
Line range hint
127-127
: ShellCheck can't follow non-constant source. Use a directive to specify location.- source "$CONFIG_FILE" + . "$CONFIG_FILE"
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
public/gm/cca-2.png
is excluded by!**/*.png
public/gm/cca-3.png
is excluded by!**/*.png
public/gm/cca.png
is excluded by!**/*.png
Files selected for processing (3)
- public/install-go.sh (3 hunks)
- public/install-yarn.sh (1 hunks)
- tutorials/gm-world-frontend.md (1 hunks)
Files not reviewed due to errors (2)
- public/install-yarn.sh (no review received)
- tutorials/gm-world-frontend.md (no review received)
Additional Context Used
LanguageTool (1)
tutorials/gm-world-frontend.md (1)
Near line 17: Possible missing comma found.
Context: ...orld) tutorial. Your rollup needs to be running since the app will connect to it via RP...
ShellCheck (11)
public/install-go.sh (3)
[style] 35-35: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
[style] 37-37: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.
[warning] 127-127: ShellCheck can't follow non-constant source. Use a directive to specify location.
public/install-yarn.sh (8)
[warning] 31-31: In POSIX sh, 'source' in place of '.' is undefined.
[warning] 31-31: ShellCheck can't follow non-constant source. Use a directive to specify location.
[info] 34-34: Double quote to prevent globbing and word splitting.
[info] 37-37: Double quote to prevent globbing and word splitting.
[info] 42-42: Double quote to prevent globbing and word splitting.
[info] 46-46: Double quote to prevent globbing and word splitting.
[info] 46-46: Double quote to prevent globbing and word splitting.
[info] 46-46: Double quote to prevent globbing and word splitting.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes