feat: add --skip-build-number-bump flag#524
Conversation
The variable now includes skipBuildNumberBump alongside output options, so the old name was misleading.
📝 WalkthroughWalkthroughIntroduces support for a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
This pull request adds a --skip-build-number-bump flag to the build credential and request commands, allowing users to opt out of automatic build number/version code incrementing. When set, builds will use the version already present in project files instead of auto-incrementing.
Changes:
- Added SKIP_BUILD_NUMBER_BUMP credential field to schema and credential pipeline
- Registered --skip-build-number-bump CLI option on build request, credentials save, and credentials update commands
- Implemented informational logging during credentials save about auto-bump status (enabled/disabled)
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/schemas/build.ts | Added SKIP_BUILD_NUMBER_BUMP to buildCredentialsSchema and skipBuildNumberBump to buildRequestOptionsSchema |
| src/index.ts | Registered --skip-build-number-bump option on build request, credentials save, and credentials update commands |
| src/build/request.ts | Added logic to pass skipBuildNumberBump flag through CLI credentials to build request |
| src/build/credentials.ts | Added environment variable loading for SKIP_BUILD_NUMBER_BUMP |
| src/build/credentials-command.ts | Implemented flag handling in save and update commands with informational logging; updated cross-platform options detection |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Normalize env var with parseOptionalBoolean (credentials.ts) - Use !== undefined check in save, update, and request commands so users can re-enable auto-increment via --no-skip-build-number-bump - Remove log on explicit set, keep only default log (PR #521 pattern) - Add --no-skip-build-number-bump option to all three commands
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/build/request.ts (1)
962-964: Same misleadingparseOptionalBooleanerror message as incredentials.tsIf
options.skipBuildNumberBumpwere somehow a non-bool string with an unrecognised value, the thrown error would still referenceoutput-upload. Apply the sameoptionNameparameterisation fix suggested forcredentials.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/build/request.ts` around lines 962 - 964, The code sets cliCredentials.SKIP_BUILD_NUMBER_BUMP using parseOptionalBoolean(options.skipBuildNumberBump) which can throw an error referencing the wrong option name; update the call to pass the option name so the thrown error mentions the correct flag (e.g., call parseOptionalBoolean with the second parameter 'skipBuildNumberBump'), keeping the ternary assignment to cliCredentials.SKIP_BUILD_NUMBER_BUMP unchanged and targeting the parseOptionalBoolean invocation that processes options.skipBuildNumberBump.
🧹 Nitpick comments (4)
src/build/credentials.ts (1)
240-242: Misleading error message fromparseOptionalBooleanwhen called forSKIP_BUILD_NUMBER_BUMP
parseOptionalBoolean(line 89 ofcredentials.ts) throws:'output-upload must be true/false (examples: --output-upload, --output-upload false)'If a user sets
SKIP_BUILD_NUMBER_BUMP=maybe, they'll see an error mentioningoutput-upload, which is confusing. This same pre-existing issue now affects a second caller.Consider parameterising the option name:
♻️ Proposed fix
-export function parseOptionalBoolean(value: boolean | string | undefined): boolean { +export function parseOptionalBoolean(value: boolean | string | undefined, optionName = 'output-upload'): boolean { if (value === undefined) return true if (typeof value === 'boolean') return value const normalized = value.trim().toLowerCase() if (normalized === 'true' || normalized === '1' || normalized === 'yes') return true if (normalized === 'false' || normalized === '0' || normalized === 'no') return false - throw new Error('output-upload must be true/false (examples: --output-upload, --output-upload false)') + throw new Error(`${optionName} must be true/false (examples: --${optionName}, --${optionName} false)`) }Then at line 241:
- credentials.SKIP_BUILD_NUMBER_BUMP = parseOptionalBoolean(skipBuildNumberBump) ? 'true' : 'false' + credentials.SKIP_BUILD_NUMBER_BUMP = parseOptionalBoolean(skipBuildNumberBump, 'skip-build-number-bump') ? 'true' : 'false'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/build/credentials.ts` around lines 240 - 242, The parseOptionalBoolean helper currently emits a hardcoded error mentioning "output-upload", so when called for SKIP_BUILD_NUMBER_BUMP it shows the wrong option name; update parseOptionalBoolean to accept an optional optionName parameter (preserving the current default to avoid breaking callers) and use that optionName in the thrown error message, then change the call in credentials.ts that sets credentials.SKIP_BUILD_NUMBER_BUMP to call parseOptionalBoolean(skipBuildNumberBump, 'skip-build-number-bump') (and update any other callers to pass an appropriate optionName) so the error references the correct flag.src/build/credentials-command.ts (3)
108-113: Noisy default-info log fires on everycredentials savethat omits the flagThe
ℹ️ --skip-build-number-bump not specified, build number will be auto-incremented (default)line will appear on everycredentials saveinvocation that doesn't explicitly pass the flag — including users who have never heard of this option. Unlike the--output-uploaddefault log (which explains a concrete change: "defaulting to false"), this message describes a server-side default and adds noise for the common case.Consider only logging when the flag is present, or suppress the info line altogether and rely on
--helpdocumentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/build/credentials-command.ts` around lines 108 - 113, The info log message about the default for --skip-build-number-bump is noisy and should not print on every credentials save; update the block that handles options.skipBuildNumberBump and credentials.SKIP_BUILD_NUMBER_BUMP so it only logs when the flag is explicitly provided (e.g., when options.skipBuildNumberBump is defined and parsed), otherwise suppress the info line entirely (remove the log.info call in the else branch) to avoid spamming users; reference the conditional using options.skipBuildNumberBump and the assignment to credentials.SKIP_BUILD_NUMBER_BUMP to locate the change.
503-505: Vague cross-platform error message leaves users guessingWhen a user runs
build credentials update --skip-build-number-bump(without--platform), they hit thehasCrossPlatformOptionsbranch, but the error message says only:"These options require --platform to be set (ios or android)."
Because
--skip-build-number-bumpdoesn't look like a platform-specific option, the message is opaque. Consider naming the triggering option:♻️ Proposed improvement
- else if (hasCrossPlatformOptions) { - log.error('These options require --platform to be set (ios or android).') + else if (hasCrossPlatformOptions) { + const which = [ + options.outputUpload !== undefined && '--output-upload', + options.outputRetention !== undefined && '--output-retention', + options.skipBuildNumberBump !== undefined && '--skip-build-number-bump', + ].filter(Boolean).join(', ') + log.error(`${which} require(s) --platform to be set (ios or android).`) exit(1) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/build/credentials-command.ts` around lines 503 - 505, The error message in the hasCrossPlatformOptions branch is too vague; update the branch handling around hasCrossPlatformOptions in credentials-command.ts so the log.error includes the specific option name(s) that triggered the cross-platform restriction (for example the --skip-build-number-bump flag) instead of a generic message; obtain the offending option names from the same detection logic used to set hasCrossPlatformOptions and include them in the error string (e.g., "Option --skip-build-number-bump requires --platform to be set (ios or android)"), then call exit(1) as before.
489-489:SKIP_BUILD_NUMBER_BUMPis a behavior flag but must be set per-platform — document the dual-run requirementBecause credentials are stored per-platform (
ios/android), a user who wants to skip build-number bumping for both platforms must runcredentials update(orcredentials save) twice — once with--platform iosand once with--platform android. This is non-obvious since the flag is not a secret and behaves the same on both platforms.Adding a log hint after saving/updating when
skipBuildNumberBumpis set would reduce confusion:if (options.skipBuildNumberBump !== undefined) { credentials.SKIP_BUILD_NUMBER_BUMP = parseOptionalBoolean(options.skipBuildNumberBump) ? 'true' : 'false' log.info('ℹ️ SKIP_BUILD_NUMBER_BUMP applies per-platform. Run credentials update for the other platform too if needed.') }Also applies to: 553-555
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/build/credentials-command.ts` at line 489, When options.skipBuildNumberBump is used update the save/update flow to both set credentials.SKIP_BUILD_NUMBER_BUMP ('true'/'false') and emit an informational log that the flag is per-platform and requires running credentials update/save for the other platform too; locate the code around the hasCrossPlatformOptions declaration (const hasCrossPlatformOptions) and the blocks that handle options.skipBuildNumberBump (including the similar occurrence further down), set credentials.SKIP_BUILD_NUMBER_BUMP from parseOptionalBoolean(options.skipBuildNumberBump) and add a log.info call that says the flag applies per-platform and to run the command again for the other platform.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/build/request.ts`:
- Around line 962-964: The code sets cliCredentials.SKIP_BUILD_NUMBER_BUMP using
parseOptionalBoolean(options.skipBuildNumberBump) which can throw an error
referencing the wrong option name; update the call to pass the option name so
the thrown error mentions the correct flag (e.g., call parseOptionalBoolean with
the second parameter 'skipBuildNumberBump'), keeping the ternary assignment to
cliCredentials.SKIP_BUILD_NUMBER_BUMP unchanged and targeting the
parseOptionalBoolean invocation that processes options.skipBuildNumberBump.
---
Nitpick comments:
In `@src/build/credentials-command.ts`:
- Around line 108-113: The info log message about the default for
--skip-build-number-bump is noisy and should not print on every credentials
save; update the block that handles options.skipBuildNumberBump and
credentials.SKIP_BUILD_NUMBER_BUMP so it only logs when the flag is explicitly
provided (e.g., when options.skipBuildNumberBump is defined and parsed),
otherwise suppress the info line entirely (remove the log.info call in the else
branch) to avoid spamming users; reference the conditional using
options.skipBuildNumberBump and the assignment to
credentials.SKIP_BUILD_NUMBER_BUMP to locate the change.
- Around line 503-505: The error message in the hasCrossPlatformOptions branch
is too vague; update the branch handling around hasCrossPlatformOptions in
credentials-command.ts so the log.error includes the specific option name(s)
that triggered the cross-platform restriction (for example the
--skip-build-number-bump flag) instead of a generic message; obtain the
offending option names from the same detection logic used to set
hasCrossPlatformOptions and include them in the error string (e.g., "Option
--skip-build-number-bump requires --platform to be set (ios or android)"), then
call exit(1) as before.
- Line 489: When options.skipBuildNumberBump is used update the save/update flow
to both set credentials.SKIP_BUILD_NUMBER_BUMP ('true'/'false') and emit an
informational log that the flag is per-platform and requires running credentials
update/save for the other platform too; locate the code around the
hasCrossPlatformOptions declaration (const hasCrossPlatformOptions) and the
blocks that handle options.skipBuildNumberBump (including the similar occurrence
further down), set credentials.SKIP_BUILD_NUMBER_BUMP from
parseOptionalBoolean(options.skipBuildNumberBump) and add a log.info call that
says the flag applies per-platform and to run the command again for the other
platform.
In `@src/build/credentials.ts`:
- Around line 240-242: The parseOptionalBoolean helper currently emits a
hardcoded error mentioning "output-upload", so when called for
SKIP_BUILD_NUMBER_BUMP it shows the wrong option name; update
parseOptionalBoolean to accept an optional optionName parameter (preserving the
current default to avoid breaking callers) and use that optionName in the thrown
error message, then change the call in credentials.ts that sets
credentials.SKIP_BUILD_NUMBER_BUMP to call
parseOptionalBoolean(skipBuildNumberBump, 'skip-build-number-bump') (and update
any other callers to pass an appropriate optionName) so the error references the
correct flag.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/build/credentials-command.tssrc/build/credentials.tssrc/build/request.tssrc/index.tssrc/schemas/build.ts
Summary
--skip-build-number-bumpflag tobuild request,build credentials save, andbuild credentials updatecommandsSKIP_BUILD_NUMBER_BUMPenvironment variableBUILD_OUTPUT_UPLOAD_ENABLED)credentials saveabout whether auto-bump is enabled or disabled (matching PR feat: default BUILD_OUTPUT_UPLOAD_ENABLED to false, log defaults #521 pattern)Files changed
src/schemas/build.ts— addedSKIP_BUILD_NUMBER_BUMPto credentials schema + request options schemasrc/index.ts— registered--skip-build-number-bumpoption on build request, credentials save, and credentials update commandssrc/build/credentials-command.ts— wired flag through save/update logic with loggingsrc/build/request.ts— passes flag through build request credentialssrc/build/credentials.ts— readsSKIP_BUILD_NUMBER_BUMPfrom environment variablesCompanion PR
Test plan
build credentials save --platform android --keystore ... --skip-build-number-bumpsaves SKIP_BUILD_NUMBER_BUMP=true, logs itbuild credentials save --platform android --keystore ...(without flag) logs default (auto-bump enabled)build credentials update --skip-build-number-bumpupdates only that fieldbuild request --platform ios --skip-build-number-bumppasses through to credentialsSKIP_BUILD_NUMBER_BUMP=truepicked up during credential mergingSummary by CodeRabbit
--skip-build-number-bumpand--no-skip-build-number-bumpCLI flags to build request and credentials save commands. Users can now explicitly control whether build numbers are automatically incremented during builds. The preference is stored and applied consistently across all future builds.