-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: generate typescript and publish on merge #143
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis update introduces a workflow for publishing TypeScript code, enhancing the project's automation capabilities. It adds and modifies recipes in the Changes
Assessment against linked issues
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- .github/workflows/publish-ts.yml (1 hunks)
- justfile (2 hunks)
Additional comments: 3
.github/workflows/publish-ts.yml (1)
- 3-28: The workflow is well-structured and follows best practices for GitHub Actions. However, consider the following points for improvement:
- Security: Ensure that
DEPLOY_KEY
is securely stored as a GitHub secret and has the minimal required access rights on the deployment server.- Node.js Version: Using a specific version of Node.js (
"14"
) is good for consistency. However, verify if this version aligns with the project's current Node.js version requirements to avoid potential compatibility issues.- rsync Usage: The
rsync
command uses-avz
flags for verbose, archive mode, and compression. Ensure that verbose mode (-v
) does not leak sensitive information in the logs. Additionally, consider specifying the exact directories or files to sync to avoid unintentional overwrites.justfile (2)
- 4-4: Adding
set dotenv-load
is a good practice for managing environment variables. Ensure that the.env
file is properly secured and not checked into version control to prevent accidental exposure of sensitive information.- 121-137: The
gen-ts
recipe introduces a new process for generating TypeScript code. While the approach is generally sound, consider the following points:
- Schema Directory Handling: The script assumes the presence of a
schema
directory. Ensure that this directory is created or checked for existence before proceeding with the TypeScript generation to avoid potential errors.- Error Handling: Similar to the
gen-schema
recipe, ensure proper error handling and exit codes for each step of the process to maintain robustness in automated environments.
initial_dir=$PWD | ||
for dir in contracts/*; do | ||
dir_name=$(basename $dir) | ||
echo "Generating schema for $dir_name..." | ||
|
||
# Change to the contract directory | ||
if cd $dir; then | ||
# Check if 'cargo schema' can be run successfully | ||
if cargo schema; then | ||
# Move back to the initial directory | ||
cd $initial_dir | ||
# Create target schema directory if it doesn't exist | ||
mkdir -p schema/$dir_name | ||
# Move the generated schema to the target directory | ||
if ! mv $dir/schema schema/$dir_name; then | ||
echo "Failed to move schema directory for $dir_name." | ||
fi | ||
else | ||
cd $initial_dir | ||
fi | ||
else | ||
echo "Failed to change directory to $dir." | ||
fi | ||
done |
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.
The gen-schema
recipe has been enhanced to handle directory changes and schema generation more robustly. Consider the following improvements:
- Error Handling: Ensure that the script exits with a non-zero status code when encountering errors to prevent silent failures in CI/CD pipelines.
- Directory Navigation: Use
pushd
andpopd
instead ofcd
for changing directories. This approach is more reliable and readable, especially in scripts with multiple directory changes.
- if cd $dir; then
+ if pushd $dir > /dev/null; then
- cd $initial_dir
+ popd > /dev/null
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
#!/usr/bin/env bash | |
initial_dir=$PWD | |
for dir in contracts/*; do | |
dir_name=$(basename $dir) | |
echo "Generating schema for $dir_name..." | |
# Change to the contract directory | |
if cd $dir; then | |
# Check if 'cargo schema' can be run successfully | |
if cargo schema; then | |
# Move back to the initial directory | |
cd $initial_dir | |
# Create target schema directory if it doesn't exist | |
mkdir -p schema/$dir_name | |
# Move the generated schema to the target directory | |
if ! mv $dir/schema schema/$dir_name; then | |
echo "Failed to move schema directory for $dir_name." | |
fi | |
else | |
cd $initial_dir | |
fi | |
else | |
echo "Failed to change directory to $dir." | |
fi | |
done | |
#!/usr/bin/env bash | |
initial_dir=$PWD | |
for dir in contracts/*; do | |
dir_name=$(basename $dir) | |
echo "Generating schema for $dir_name..." | |
# Change to the contract directory | |
if pushd $dir > /dev/null; then | |
# Check if 'cargo schema' can be run successfully | |
if cargo schema; then | |
# Move back to the initial directory | |
popd > /dev/null | |
# Create target schema directory if it doesn't exist | |
mkdir -p schema/$dir_name | |
# Move the generated schema to the target directory | |
if ! mv $dir/schema schema/$dir_name; then | |
echo "Failed to move schema directory for $dir_name." | |
fi | |
else | |
popd > /dev/null | |
fi | |
else | |
echo "Failed to change directory to $dir." | |
fi | |
done |
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.
Fix #85 85
Summary by CodeRabbit
New Features
Chores