docs: fix outdated docs for updating secrets#76
Conversation
WalkthroughUpdated DEVELOPMENT.md to replace Firebase CLI configuration commands with secret management commands for storing credentials (DISCORD_TOKEN, GITHUB_CLIENT_SECRET, GITHUB_PRIVATE_KEY, INTERNAL_TOKEN). Also modified firebase.json predeploy script by adding "run" argument to the test command. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
DEVELOPMENT.md (1)
208-217: Docs: clarify “secrets” vs “env vars/config”, and mention binding + deploy step.This section says “Update environment variables”, but the commands are Secret Manager-backed secrets. Also, consider adding one line that functions must be configured to consume these secrets (and that you’ll need to deploy/redeploy functions after setting/rotating them).
-## Firebase Configuration - -Update environment variables in Firebase (useful when rotating tokens or migrating environments): +## Firebase Secrets + +Set/rotate Firebase Functions secrets (useful when rotating tokens or migrating environments). Redeploy functions after changes. ```bash firebase functions:secrets:set DISCORD_TOKEN firebase functions:secrets:set GITHUB_CLIENT_SECRET firebase functions:secrets:set GITHUB_PRIVATE_KEY firebase functions:secrets:set INTERNAL_TOKEN
+> Note: Ensure your Functions code is configured to access these secrets (per Firebase Functions secrets docs / your runtime generation).
</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 04081eb8b558206021bb1ef3261a97cd4a772d49 and 3223cba26341c91305e0a0faea12ccb9713bf689. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `DEVELOPMENT.md` (1 hunks) * `firebase.json` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (1)</summary> <details> <summary>📚 Learning: 2025-10-09T00:43:18.517Z</summary>Learnt from: webbertakken
Repo: game-ci/versioning-backend PR: 69
File: functions/package.json:6-17
Timestamp: 2025-10-09T00:43:18.517Z
Learning: In the game-ci/versioning-backend repository, therun -Tpattern used in package.json scripts (e.g.,"build": "run -T tsc") is valid Yarn syntax and works as intended. This should not be flagged as an error requiringyarn run -Tprefix.**Applied to files:** - `firebase.json` </details> </details> </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| "predeploy": [ | ||
| "yarn workspace functions lint", | ||
| "yarn workspace functions test", | ||
| "yarn workspace functions test run", | ||
| "yarn workspace functions build" | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the Yarn version configuration
find . -type f \( -name "package.json" -o -name ".yarnrc*" -o -name "yarn.lock" \) | head -20Repository: game-ci/versioning-backend
Length of output: 135
🏁 Script executed:
# Check the root package.json for Yarn version
head -30 package.jsonRepository: game-ci/versioning-backend
Length of output: 946
🏁 Script executed:
# Look at the functions/package.json to see how test script is defined
find . -path "*/functions/package.json" -exec cat {} \;Repository: game-ci/versioning-backend
Length of output: 1583
🏁 Script executed:
# Check if there's a .yarnrc or .yarnrc.yml
find . -name ".yarnrc*" -exec cat {} \;Repository: game-ci/versioning-backend
Length of output: 144
Fix broken predeploy test command.
yarn workspace functions test run will pass run as an argument to vitest instead of executing the intended test script. Use yarn workspace functions test to run the test script from the functions workspace.
"predeploy": [
"yarn workspace functions lint",
- "yarn workspace functions test run",
+ "yarn workspace functions test",
"yarn workspace functions build"
]📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "predeploy": [ | |
| "yarn workspace functions lint", | |
| "yarn workspace functions test", | |
| "yarn workspace functions test run", | |
| "yarn workspace functions build" | |
| ] | |
| "predeploy": [ | |
| "yarn workspace functions lint", | |
| "yarn workspace functions test", | |
| "yarn workspace functions build" | |
| ] |
🤖 Prompt for AI Agents
In firebase.json around lines 8 to 12, the predeploy hook uses "yarn workspace
functions test run" which incorrectly passes "run" as an argument to the test
runner; change that entry to use "yarn workspace functions test" so the
functions workspace's test script is executed directly (leave the lint and build
steps unchanged).
There was a problem hiding this comment.
@coderabbitai no, this is as intended, because in this context it's supposed to run non-interactive
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
vitest run command non-interactive mode
💡 Result:
Use Vitest's run mode (non‑interactive) — either the command or the flag:
- Run once (no watch / no interactive):
- vitest run
- or vitest --run
Vitest also automatically falls back to run (non‑interactive) when CI=true or when stdin is not a TTY. Example for CI:
CI=true vitest
Sources: Vitest CLI docs. [1][2]
References
- Vitest CLI — commands and
vitest run. [vitest.dev CLI]. [2] - Guide — watch vs run and CI/TTY behavior. [vitest.dev features]. [1]
@webbertakken, you're absolutely right, and thank you for the clarification! I apologize for the confusion.
The command yarn workspace functions test run correctly passes run as an argument to vitest (via the test script "test": "run -T vitest"), which runs vitest in non-interactive mode. This is exactly what you want in a predeploy hook context where tests should run once and exit, rather than entering watch mode.
I've noted this for future reviews.
✏️ Learnings added
Learnt from: webbertakken
Repo: game-ci/versioning-backend PR: 76
File: firebase.json:8-12
Timestamp: 2025-12-14T00:46:32.906Z
Learning: In the game-ci/versioning-backend repository, the pattern "yarn workspace functions test run" in firebase.json's predeploy hooks is intentional. It passes "run" to vitest (via the test script "run -T vitest") to execute tests in non-interactive mode, which is appropriate for CI/predeploy contexts.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: webbertakken
Repo: game-ci/versioning-backend PR: 69
File: functions/package.json:6-17
Timestamp: 2025-10-09T00:43:18.517Z
Learning: In the game-ci/versioning-backend repository, the `run -T` pattern used in package.json scripts (e.g., `"build": "run -T tsc"`) is valid Yarn syntax and works as intended. This should not be flagged as an error requiring `yarn run -T` prefix.
There was a problem hiding this comment.
@webbertakken, thank you for confirming! I appreciate your patience in helping me understand the intended behavior of the test command in your predeploy hooks. 🙏
🧠 Learnings used
Learnt from: webbertakken
Repo: game-ci/versioning-backend PR: 76
File: firebase.json:8-12
Timestamp: 2025-12-14T00:46:32.906Z
Learning: In the game-ci/versioning-backend repository, the pattern "yarn workspace functions test run" in firebase.json's predeploy hooks is intentional. It passes "run" to vitest (via the test script "run -T vitest") to execute tests in non-interactive mode, which is appropriate for CI/predeploy contexts.
Learnt from: webbertakken
Repo: game-ci/versioning-backend PR: 69
File: functions/package.json:6-17
Timestamp: 2025-10-09T00:43:18.517Z
Learning: In the game-ci/versioning-backend repository, the `run -T` pattern used in package.json scripts (e.g., `"build": "run -T tsc"`) is valid Yarn syntax and works as intended. This should not be flagged as an error requiring `yarn run -T` prefix.
2 participants
Fixes were done in #75
This is a follow-up updating the docs
Changes
Checklist
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.