added render_pi_env.sh#155
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new script, scripts/render_pi_env.sh, which automates the generation of environment files for the elisa, elisheba, and isabel services by fetching secrets from 1Password. The reviewer suggested improving the script's maintainability by extracting hardcoded configuration values into variables and optimizing performance by using op inject to batch secret retrieval instead of making multiple individual calls to op read.
| output_dir="${1:-build/pi-env}" | ||
| op_account="UKDQEQIPJVEARKGQPBMCLFP5MQ" |
There was a problem hiding this comment.
Hardcoding environment-specific values like the MQTT address and device IPs directly in the heredocs makes the script harder to maintain. It is recommended to define these as variables at the top of the script.
| output_dir="${1:-build/pi-env}" | |
| op_account="UKDQEQIPJVEARKGQPBMCLFP5MQ" | |
| output_dir="${1:-build/pi-env}" | |
| op_account="UKDQEQIPJVEARKGQPBMCLFP5MQ" | |
| mqtt_address="mqtts://lisa.chipp.dev:8880" | |
| roborock_ip="10.0.1.150" |
| read_secret() { | ||
| local ref="$1" | ||
|
|
||
| op read --account "$op_account" "$ref" | ||
| } |
There was a problem hiding this comment.
Calling op read multiple times is inefficient because each invocation starts a new process and performs authentication. A more performant and idiomatic approach is to use op inject with templates (e.g., cat <<EOF | op inject ...), which allows fetching all secrets in a single batch operation.
1 participant
No description provided.