Conversation
📝 WalkthroughWalkthroughTwo small configuration adjustments: adding a Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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)
src/services/bluetooth-audio.service.ts (1)
1474-1494: Polling at 700 ms doubles BLE read frequency — confirm the trade-off is intentional.Reducing from 1500 ms to 700 ms halves the average button-detection latency (from ~750 ms to ~350 ms), which is a real improvement for PTT responsiveness. The
isReadPollingInFlightguard correctly prevents re-entrant reads, so slow BLE reads won't pile up. The new interval also fits neatly into the 1400 ms PTT-release fallback window (schedulePttReleaseFallback), allowing up to 2 full poll cycles before the fallback fires.The main cost is roughly double the BLE radio activity for every monitored characteristic. On iOS specifically, Apple recommends notifications over polling; while 700 ms is well within Core Bluetooth's allowed read rate, the increased frequency may marginally affect battery life during long active sessions.
If this was tuned by testing on real devices, no action needed — consider making the interval a named constant for clarity.
♻️ Optional: extract interval as a named constant
+ private static readonly READ_POLLING_INTERVAL_MS = 700; ... - }, 700); + }, BluetoothAudioService.READ_POLLING_INTERVAL_MS);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/bluetooth-audio.service.ts` around lines 1474 - 1494, The polling interval of 700ms is a magic number affecting BLE read frequency; extract it into a clearly named constant (e.g., HEADSET_BUTTON_POLL_INTERVAL_MS) used where setInterval is created in the read polling logic (referencing readPollingInterval, isReadPollingInFlight, pollReadCharacteristics, stopReadPollingFallback and schedulePttReleaseFallback) so the trade-off is explicit and easy to adjust; ensure any tests or comments note the rationale for the chosen value and replace the literal 700 with the new constant everywhere in this polling code path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/react-native-cicd.yml:
- Line 81: The workflow sets YARN_PRODUCTION: 'false' which may not prevent
devDependencies being skipped if EAS CLI invokes yarn install with
NODE_ENV=production; verify EAS CLI's install behavior and if it calls yarn
directly, change the workflow to either set NODE_ENV: 'development' or add the
explicit yarn CLI flag --production=false to the install step (or ensure EAS
uses buildpack-style pruning if you want YARN_PRODUCTION to apply); look for the
CI step that runs yarn install or invokes EAS CLI and update it to pass
--production=false or set NODE_ENV accordingly.
---
Nitpick comments:
In `@src/services/bluetooth-audio.service.ts`:
- Around line 1474-1494: The polling interval of 700ms is a magic number
affecting BLE read frequency; extract it into a clearly named constant (e.g.,
HEADSET_BUTTON_POLL_INTERVAL_MS) used where setInterval is created in the read
polling logic (referencing readPollingInterval, isReadPollingInFlight,
pollReadCharacteristics, stopReadPollingFallback and schedulePttReleaseFallback)
so the trade-off is explicit and easy to adjust; ensure any tests or comments
note the rationale for the chosen value and replace the literal 700 with the new
constant everywhere in this polling code path.
| NODE_OPTIONS: --openssl-legacy-provider | ||
| CHANGERAWR_API_KEY: ${{ secrets.CHANGERAWR_API_KEY }} | ||
| CHANGERAWR_API_URL: ${{ secrets.CHANGERAWR_API_URL }} | ||
| YARN_PRODUCTION: 'false' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the Yarn version and whether YARN_PRODUCTION is respected in this project.
# 1. Find the declared Yarn version (package.json engines or .yarnrc)
echo "=== package.json engines field ==="
fd -g 'package.json' --max-depth 1 --exec grep -A3 '"engines"' {} \;
echo ""
echo "=== .yarnrc or .yarnrc.yml ==="
fd -g '.yarnrc*' --max-depth 2 --exec cat {} \;
echo ""
echo "=== yarn.lock format (v1 has '# yarn lockfile v1') ==="
head -5 yarn.lock 2>/dev/null || echo "yarn.lock not found at root"Repository: Resgrid/Unit
Length of output: 508
🏁 Script executed:
#!/bin/bash
# Rerun with -H flag for hidden files and get more version info
echo "=== .yarnrc or .yarnrc.yml (with hidden files) ==="
fd -H -g '.yarnrc*' --max-depth 2 --exec cat {} \; 2>/dev/null || echo "No .yarnrc files found"
echo ""
echo "=== Full yarn.lock header ==="
head -10 yarn.lock 2>/dev/null
echo ""
echo "=== package.json full content (check for yarn version anywhere) ==="
cat -n package.json | head -50Repository: Resgrid/Unit
Length of output: 4430
🌐 Web query:
Yarn v1 YARN_PRODUCTION environment variable NODE_ENV production
💡 Result:
In Yarn v1 (Classic):
NODE_ENV=productionmakesyarn installskipdevDependenciesby default. The Classic docs state: “Yarn will not install any package listed indevDependenciesif theNODE_ENVenvironment variable is set toproduction.” [1]yarn install --production/--prodis how you force/override that behavior regardless ofNODE_ENV(the docs: “Use this flag to instruct Yarn to ignoreNODE_ENVand take its production-or-not status from this flag instead.”). [1]
About YARN_PRODUCTION:
YARN_PRODUCTION=trueis commonly used by Heroku’s Node.js classic buildpack to tell the build to “Only Installdependencies…YARN_PRODUCTION=truefor Yarn v1.” [2]- Even with production installs, Yarn may still resolve
devDependenciesduring dependency-tree resolution (which can look like it’s “installing” them in logs), but Heroku notes Yarn “isn’t installing thedevDependencies” in that case. [3]
Practical combos
- Local / generic CI:
NODE_ENV=production yarn install(oryarn install --prod). - Heroku classic buildpack: set
YARN_PRODUCTION=true(and/orNPM_CONFIG_PRODUCTION=true) per Heroku docs. [2]
Sources
- https://classic.yarnpkg.com/lang/en/docs/cli/install/
- https://devcenter.heroku.com/articles/nodejs-classic-buildpack-builds
- https://help.heroku.com/AHO7DWSQ/why-is-yarn-installing-my-devdependencies-when-i-am-using-yarn_production-true
🌐 Web query:
YARN_PRODUCTION=false NODE_ENV=production Yarn v1 override precedence
💡 Result:
In Yarn v1 (Classic), the “production install” decision (i.e., whether to skip devDependencies) works like this:
-
If you pass
yarn install --production[=true|false], that flag wins and Yarn ignoresNODE_ENVfor this decision. So--production=falseforces devDependencies to be installed even ifNODE_ENV=production. [1] -
If you do not pass
--production, thenNODE_ENV=productionmakes Yarn skipdevDependenciesby default. [1]
Separately (and this is often where the confusion comes from): on platforms that do an extra “prune devDependencies” build step (e.g., Heroku’s classic Node buildpack / DigitalOcean App Platform buildpacks), setting:
YARN_PRODUCTION=falseskips that pruning step even ifNODE_ENV=production, effectively letting devDependencies remain installed in the slug/image while keepingNODE_ENV=production. [2][3]
So with YARN_PRODUCTION=false NODE_ENV=production:
- Yarn v1 itself will still treat
NODE_ENV=productionas production unless you also pass--production=false. [1] - Buildpack pruning, if present, is typically overridden/disabled by
YARN_PRODUCTION=false. [2][3]
Sources:
[1] Yarn Classic yarn install --production[=true|false] docs (explains --production overrides/ignores NODE_ENV). (classic.yarnpkg.com)
[2] Heroku “Node.js Classic Buildpack Builds” (Skip pruning using Yarn v1: YARN_PRODUCTION=false). (devcenter.heroku.com)
[3] DigitalOcean App Platform Node.js buildpack docs (Skip pruning: YARN_PRODUCTION=false for Yarn v1). (docs.digitalocean.com)
Citations:
- 1: https://classic.yarnpkg.com/en/docs/cli/install?utm_source=openai
- 2: https://devcenter.heroku.com/articles/nodejs-classic-buildpack-builds?utm_source=openai
- 3: https://docs.digitalocean.com/products/app-platform/reference/buildpacks/nodejs/?utm_source=openai
YARN_PRODUCTION=false may not override NODE_ENV=production during Yarn v1 install—confirm EAS CLI behavior and consider alternative fix.
The concern about NODE_ENV=production skipping devDependencies is valid. However, YARN_PRODUCTION=false is designed for buildpack-level pruning steps (Heroku, DigitalOcean), not for Yarn v1's own install behavior. In Yarn v1, only the --production=false CLI flag reliably overrides NODE_ENV=production during install. If EAS CLI internally sets NODE_ENV=production and calls yarn install without the --production=false flag, then YARN_PRODUCTION=false alone will not prevent devDependencies from being skipped.
Verify whether EAS CLI uses a buildpack-style pruning approach (in which case YARN_PRODUCTION=false helps) or directly invokes Yarn install with NODE_ENV=production (in which case NODE_ENV=development or explicit --production=false would be more reliable).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/react-native-cicd.yml at line 81, The workflow sets
YARN_PRODUCTION: 'false' which may not prevent devDependencies being skipped if
EAS CLI invokes yarn install with NODE_ENV=production; verify EAS CLI's install
behavior and if it calls yarn directly, change the workflow to either set
NODE_ENV: 'development' or add the explicit yarn CLI flag --production=false to
the install step (or ensure EAS uses buildpack-style pruning if you want
YARN_PRODUCTION to apply); look for the CI step that runs yarn install or
invokes EAS CLI and update it to pass --production=false or set NODE_ENV
accordingly.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
1 participant
Summary by CodeRabbit
Bug Fixes
Chores