fix(web): use REEARTH_ prefix for frontend runtime envvars
#37
Conversation
Signed-off-by: KeisukeYamashita <[email protected]>
KeisukeYamashita
requested review from
airslice and
mkumbobeaty
as code owners
WalkthroughThe script Changes
Suggested reviewers
Poem
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 (
|
✅ Deploy Preview for reearth-classic ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
web/docker/40-envsubst-on-reearth-config.sh (1)
Line range hint
26-34: Document the environment variable prefix change.Consider adding a comment above the
wrap_reearth_variablesfunction to document that it processes allREEARTH_prefixed variables for frontend configuration.+# Processes all REEARTH_ prefixed environment variables for frontend configuration. +# Values are wrapped in quotes unless they are empty, null, true, false, or JSON formatted. wrap_reearth_variables() {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
web/docker/40-envsubst-on-reearth-config.sh(1 hunks)
🔇 Additional comments (1)
web/docker/40-envsubst-on-reearth-config.sh (1)
27-27: LGTM, but verify template compatibility.
The change from REEARTH_WEB_ to REEARTH_ prefix aligns with the standardization goal. However, we should ensure this broader scope doesn't introduce unintended side effects.
Let's verify the template's expected variables and potential conflicts:
✅ Verification successful
The change from REEARTH_WEB_ to REEARTH_ prefix is safe and verified.
The template file web/docker/reearth_config.json.template shows all expected environment variables are already using the REEARTH_ prefix. The change aligns with the existing configuration structure and doesn't introduce any conflicts. The script correctly processes all necessary environment variables for the frontend configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check template file for expected variables and identify all REEARTH_ prefixed vars
# Check what variables the template expects
echo "Variables expected by template:"
if [ -f "/opt/reearth/reearth_config.json.template" ]; then
grep -o '\$REEARTH_[A-Z_]*' "/opt/reearth/reearth_config.json.template" | sort -u
fi
# List all REEARTH_ environment variables in the codebase
echo -e "\nAll REEARTH_ prefixed variables in codebase:"
rg '\$REEARTH_[A-Z_]*' --type sh --no-filename | grep -o '\$REEARTH_[A-Z_]*' | sort -u
Length of output: 357
Script:
#!/bin/bash
# Let's check the template file content and location
echo "Searching for template files:"
fd -t f "config.*template"
echo -e "\nChecking config generation script:"
cat web/docker/40-envsubst-on-reearth-config.sh
echo -e "\nChecking for any other REEARTH_ environment variable usage:"
rg "REEARTH_" -B2 -A2
Length of output: 52481
Overview
Currently, the runtime is using the
REEARTH_WEB_prefix.We have decided to use
REEARTH_instead .What I've done
Migrated to respect runtime environment variables to pick up
REEARTH_prefixed environment variables.What I haven't done
How I tested
Which point I want you to review particularly
Memo
Summary by CodeRabbit
REEARTH_.