CI: move build to release workflow#158
Conversation
- Remove lib/ from git tracking - Remove bob build from pre-commit hook - Add build step to PR checks - Create release workflow that builds and publishes to npm - Update release-it to skip npm publish (handled by GitHub Actions) - Update CONTRIBUTING.md with new build/release process
thomasttvo
force-pushed
the
ci/build-on-release
branch
from
fc52c47 to
1df0fac
Compare
- Update to latest action versions (checkout@v4, setup-node@v4) - Change release trigger from 'created' to 'published' - Add permissions block for provenance support - Add --provenance and --access public flags to npm publish - Use Node.js 20.x version format
- Remove NPM_TOKEN secret requirement - Publish uses GitHub's trusted publishing (OIDC) - No token rotation needed - Update docs to reflect OIDC setup
thomasttvo
changed the title
thomasttvo
changed the title
|
@claude please review this PR |
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
thomasttvo
changed the title
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
WalkthroughRemoved the built outputs under 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)
package.json (1)
25-30: Optional DRY cleanup for build scripts.Lines 25 and 29 duplicate the same command. Consider chaining
preparetobuildto avoid script drift.♻️ Suggested refactor
- "prepare": "bob build", + "prepare": "yarn build", ... - "build": "bob build", + "build": "bob build",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 25 - 30, The package.json duplicates the bob build command in both the "prepare" and "build" npm scripts; change "prepare" to invoke the "build" script instead (e.g., run npm script chaining like "npm run build" or "yarn build") so "prepare" delegates to the "build" script and avoids drift—update the "prepare" script entry that currently references bob build to call the "build" script instead, leaving the "build" script (bob build) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CONTRIBUTING.md`:
- Around line 93-100: The release config in package.json allows local npm
publishes (release-it.npm.publish: true) which contradicts docs; update
package.json to disable local npm publishing by setting the release-it
npm.publish flag to false (or removing it) so publishing only occurs via
CI/GitHub Actions, and ensure release-it config remains present under the
"release-it" key (reference: the release-it.npm.publish setting in package.json)
so local runs of yarn release cannot push to npm.
---
Nitpick comments:
In `@package.json`:
- Around line 25-30: The package.json duplicates the bob build command in both
the "prepare" and "build" npm scripts; change "prepare" to invoke the "build"
script instead (e.g., run npm script chaining like "npm run build" or "yarn
build") so "prepare" delegates to the "build" script and avoids drift—update the
"prepare" script entry that currently references bob build to call the "build"
script instead, leaving the "build" script (bob build) intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c80e3368-cbea-441a-aaf6-72541604c851
⛔ Files ignored due to path filters (26)
lib/commonjs/ReactNativeZoomableView.js.mapis excluded by!**/*.maplib/commonjs/animations/index.js.mapis excluded by!**/*.maplib/commonjs/assets/pin.pngis excluded by!**/*.png,!**/*.pnglib/commonjs/components/AnimatedTouchFeedback.js.mapis excluded by!**/*.maplib/commonjs/components/StaticPin.js.mapis excluded by!**/*.maplib/commonjs/components/index.js.mapis excluded by!**/*.maplib/commonjs/debugHelper/index.js.mapis excluded by!**/*.maplib/commonjs/helper/applyPanBoundariesToOffset.js.mapis excluded by!**/*.maplib/commonjs/helper/calcNewScaledOffsetForZoomCentering.js.mapis excluded by!**/*.maplib/commonjs/helper/coordinateConversion.js.mapis excluded by!**/*.maplib/commonjs/helper/index.js.mapis excluded by!**/*.maplib/commonjs/index.js.mapis excluded by!**/*.maplib/commonjs/typings/index.js.mapis excluded by!**/*.maplib/module/ReactNativeZoomableView.js.mapis excluded by!**/*.maplib/module/animations/index.js.mapis excluded by!**/*.maplib/module/assets/pin.pngis excluded by!**/*.png,!**/*.pnglib/module/components/AnimatedTouchFeedback.js.mapis excluded by!**/*.maplib/module/components/StaticPin.js.mapis excluded by!**/*.maplib/module/components/index.js.mapis excluded by!**/*.maplib/module/debugHelper/index.js.mapis excluded by!**/*.maplib/module/helper/applyPanBoundariesToOffset.js.mapis excluded by!**/*.maplib/module/helper/calcNewScaledOffsetForZoomCentering.js.mapis excluded by!**/*.maplib/module/helper/coordinateConversion.js.mapis excluded by!**/*.maplib/module/helper/index.js.mapis excluded by!**/*.maplib/module/index.js.mapis excluded by!**/*.maplib/module/typings/index.js.mapis excluded by!**/*.map
📒 Files selected for processing (42)
.github/workflows/lint.yml.github/workflows/release.yml.gitignoreCONTRIBUTING.mdlib/commonjs/ReactNativeZoomableView.jslib/commonjs/animations/index.jslib/commonjs/components/AnimatedTouchFeedback.jslib/commonjs/components/StaticPin.jslib/commonjs/components/index.jslib/commonjs/debugHelper/index.jslib/commonjs/helper/applyPanBoundariesToOffset.jslib/commonjs/helper/calcNewScaledOffsetForZoomCentering.jslib/commonjs/helper/coordinateConversion.jslib/commonjs/helper/index.jslib/commonjs/index.jslib/commonjs/typings/index.jslib/module/ReactNativeZoomableView.jslib/module/animations/index.jslib/module/components/AnimatedTouchFeedback.jslib/module/components/StaticPin.jslib/module/components/index.jslib/module/debugHelper/index.jslib/module/helper/applyPanBoundariesToOffset.jslib/module/helper/calcNewScaledOffsetForZoomCentering.jslib/module/helper/coordinateConversion.jslib/module/helper/index.jslib/module/index.jslib/module/typings/index.jslib/typescript/ReactNativeZoomableView.d.tslib/typescript/__tests__/index.test.d.tslib/typescript/animations/index.d.tslib/typescript/components/AnimatedTouchFeedback.d.tslib/typescript/components/StaticPin.d.tslib/typescript/components/index.d.tslib/typescript/debugHelper/index.d.tslib/typescript/helper/applyPanBoundariesToOffset.d.tslib/typescript/helper/calcNewScaledOffsetForZoomCentering.d.tslib/typescript/helper/coordinateConversion.d.tslib/typescript/helper/index.d.tslib/typescript/index.d.tslib/typescript/typings/index.d.tspackage.json
💤 Files with no reviewable changes (36)
- lib/typescript/components/index.d.ts
- lib/commonjs/components/index.js
- lib/module/components/index.js
- lib/commonjs/typings/index.js
- lib/typescript/components/AnimatedTouchFeedback.d.ts
- lib/module/typings/index.js
- lib/typescript/helper/applyPanBoundariesToOffset.d.ts
- lib/module/index.js
- lib/module/animations/index.js
- lib/commonjs/helper/calcNewScaledOffsetForZoomCentering.js
- lib/commonjs/helper/applyPanBoundariesToOffset.js
- lib/module/components/StaticPin.js
- lib/module/helper/applyPanBoundariesToOffset.js
- lib/commonjs/index.js
- lib/typescript/index.d.ts
- lib/commonjs/debugHelper/index.js
- lib/module/debugHelper/index.js
- lib/commonjs/components/StaticPin.js
- lib/module/helper/calcNewScaledOffsetForZoomCentering.js
- lib/typescript/debugHelper/index.d.ts
- lib/module/components/AnimatedTouchFeedback.js
- lib/typescript/helper/index.d.ts
- lib/commonjs/ReactNativeZoomableView.js
- lib/module/helper/index.js
- lib/commonjs/animations/index.js
- lib/typescript/components/StaticPin.d.ts
- lib/typescript/animations/index.d.ts
- lib/module/helper/coordinateConversion.js
- lib/typescript/typings/index.d.ts
- lib/typescript/helper/coordinateConversion.d.ts
- lib/typescript/helper/calcNewScaledOffsetForZoomCentering.d.ts
- lib/commonjs/helper/coordinateConversion.js
- lib/module/ReactNativeZoomableView.js
- lib/commonjs/components/AnimatedTouchFeedback.js
- lib/typescript/ReactNativeZoomableView.d.ts
- lib/commonjs/helper/index.js
- Add NODE_AUTH_TOKEN to release workflow for npm registry auth - Remove incorrect OIDC authentication claims from comments and docs - Change release-it npm.publish to false (CI handles publishing) - Rename prepare to prepublishOnly (avoids breaking git-based installs) - Update CONTRIBUTING.md to document NPM_TOKEN secret requirement Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai i'm confused, how do i know whether you finished reviewing or not |
|
Since this PR already had a review triggered, check if there's already a comment from You can force a fresh full review by posting: ✅ Actions performedFull review triggered. |
|
@claude review |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.gitignore (1)
50-51: Scope ignore rule to the repository root.If the intent is only the generated top-level build folder, use
/lib/instead oflib/to avoid unintentionally ignoring nestedlibdirectories.Suggested change
- lib/ + /lib/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 50 - 51, The current .gitignore entry uses a broad "lib/" pattern which ignores any lib directory anywhere; scope it to the repository root by replacing the entry "lib/" with "/lib/" so only the top-level build folder is ignored and nested lib directories remain tracked..github/workflows/release.yml (1)
32-37: Migrate fromNPM_TOKENto npm trusted publishing (OIDC) to remove long-lived secrets.The workflow already has
id-token: writeandregistry-urlconfigured—prerequisites are in place. To migrate:
- On npmjs.com: Configure trusted publisher for this package (
@openspacelabs/react-native-zoomable-view). Specify repositoryopenspacelabs/react-native-zoomable-viewand workflowrelease.yml.- Remove
NODE_AUTH_TOKENenv var (lines 36–37); OIDC authentication will handle publishing.- Optionally remove
--provenanceflag (auto-enabled with trusted publishing and public repo).This eliminates token rotation burden and tightens supply-chain security.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 32 - 37, The workflow currently uses a long-lived token via the NODE_AUTH_TOKEN env var and an explicit publish command "npm publish --access public --provenance"; update the release.yml workflow to rely on GitHub OIDC by removing the NODE_AUTH_TOKEN environment variable and its assignment, keep (or ensure) "id-token: write" and the configured registry-url step present, and change the publish step to run "npm publish --access public" (you can also drop the "--provenance" flag if you’ve configured a trusted publisher for `@openspacelabs/react-native-zoomable-view`); ensure the release.yml workflow name/repository mapping matches the trusted publisher settings so OIDC can authenticate without a long-lived token.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 32-37: The workflow currently uses a long-lived token via the
NODE_AUTH_TOKEN env var and an explicit publish command "npm publish --access
public --provenance"; update the release.yml workflow to rely on GitHub OIDC by
removing the NODE_AUTH_TOKEN environment variable and its assignment, keep (or
ensure) "id-token: write" and the configured registry-url step present, and
change the publish step to run "npm publish --access public" (you can also drop
the "--provenance" flag if you’ve configured a trusted publisher for
`@openspacelabs/react-native-zoomable-view`); ensure the release.yml workflow
name/repository mapping matches the trusted publisher settings so OIDC can
authenticate without a long-lived token.
In @.gitignore:
- Around line 50-51: The current .gitignore entry uses a broad "lib/" pattern
which ignores any lib directory anywhere; scope it to the repository root by
replacing the entry "lib/" with "/lib/" so only the top-level build folder is
ignored and nested lib directories remain tracked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b5de9486-e972-4e5a-861b-435cd7794f90
⛔ Files ignored due to path filters (26)
lib/commonjs/ReactNativeZoomableView.js.mapis excluded by!**/*.maplib/commonjs/animations/index.js.mapis excluded by!**/*.maplib/commonjs/assets/pin.pngis excluded by!**/*.png,!**/*.pnglib/commonjs/components/AnimatedTouchFeedback.js.mapis excluded by!**/*.maplib/commonjs/components/StaticPin.js.mapis excluded by!**/*.maplib/commonjs/components/index.js.mapis excluded by!**/*.maplib/commonjs/debugHelper/index.js.mapis excluded by!**/*.maplib/commonjs/helper/applyPanBoundariesToOffset.js.mapis excluded by!**/*.maplib/commonjs/helper/calcNewScaledOffsetForZoomCentering.js.mapis excluded by!**/*.maplib/commonjs/helper/coordinateConversion.js.mapis excluded by!**/*.maplib/commonjs/helper/index.js.mapis excluded by!**/*.maplib/commonjs/index.js.mapis excluded by!**/*.maplib/commonjs/typings/index.js.mapis excluded by!**/*.maplib/module/ReactNativeZoomableView.js.mapis excluded by!**/*.maplib/module/animations/index.js.mapis excluded by!**/*.maplib/module/assets/pin.pngis excluded by!**/*.png,!**/*.pnglib/module/components/AnimatedTouchFeedback.js.mapis excluded by!**/*.maplib/module/components/StaticPin.js.mapis excluded by!**/*.maplib/module/components/index.js.mapis excluded by!**/*.maplib/module/debugHelper/index.js.mapis excluded by!**/*.maplib/module/helper/applyPanBoundariesToOffset.js.mapis excluded by!**/*.maplib/module/helper/calcNewScaledOffsetForZoomCentering.js.mapis excluded by!**/*.maplib/module/helper/coordinateConversion.js.mapis excluded by!**/*.maplib/module/helper/index.js.mapis excluded by!**/*.maplib/module/index.js.mapis excluded by!**/*.maplib/module/typings/index.js.mapis excluded by!**/*.map
📒 Files selected for processing (42)
.github/workflows/lint.yml.github/workflows/release.yml.gitignoreCONTRIBUTING.mdlib/commonjs/ReactNativeZoomableView.jslib/commonjs/animations/index.jslib/commonjs/components/AnimatedTouchFeedback.jslib/commonjs/components/StaticPin.jslib/commonjs/components/index.jslib/commonjs/debugHelper/index.jslib/commonjs/helper/applyPanBoundariesToOffset.jslib/commonjs/helper/calcNewScaledOffsetForZoomCentering.jslib/commonjs/helper/coordinateConversion.jslib/commonjs/helper/index.jslib/commonjs/index.jslib/commonjs/typings/index.jslib/module/ReactNativeZoomableView.jslib/module/animations/index.jslib/module/components/AnimatedTouchFeedback.jslib/module/components/StaticPin.jslib/module/components/index.jslib/module/debugHelper/index.jslib/module/helper/applyPanBoundariesToOffset.jslib/module/helper/calcNewScaledOffsetForZoomCentering.jslib/module/helper/coordinateConversion.jslib/module/helper/index.jslib/module/index.jslib/module/typings/index.jslib/typescript/ReactNativeZoomableView.d.tslib/typescript/__tests__/index.test.d.tslib/typescript/animations/index.d.tslib/typescript/components/AnimatedTouchFeedback.d.tslib/typescript/components/StaticPin.d.tslib/typescript/components/index.d.tslib/typescript/debugHelper/index.d.tslib/typescript/helper/applyPanBoundariesToOffset.d.tslib/typescript/helper/calcNewScaledOffsetForZoomCentering.d.tslib/typescript/helper/coordinateConversion.d.tslib/typescript/helper/index.d.tslib/typescript/index.d.tslib/typescript/typings/index.d.tspackage.json
💤 Files with no reviewable changes (36)
- lib/module/components/index.js
- lib/typescript/components/index.d.ts
- lib/module/typings/index.js
- lib/module/index.js
- lib/commonjs/typings/index.js
- lib/module/helper/applyPanBoundariesToOffset.js
- lib/typescript/components/AnimatedTouchFeedback.d.ts
- lib/typescript/helper/applyPanBoundariesToOffset.d.ts
- lib/typescript/debugHelper/index.d.ts
- lib/typescript/index.d.ts
- lib/commonjs/index.js
- lib/commonjs/helper/calcNewScaledOffsetForZoomCentering.js
- lib/typescript/helper/calcNewScaledOffsetForZoomCentering.d.ts
- lib/commonjs/animations/index.js
- lib/module/helper/calcNewScaledOffsetForZoomCentering.js
- lib/module/components/AnimatedTouchFeedback.js
- lib/module/animations/index.js
- lib/commonjs/helper/index.js
- lib/module/components/StaticPin.js
- lib/module/helper/index.js
- lib/commonjs/components/AnimatedTouchFeedback.js
- lib/module/debugHelper/index.js
- lib/typescript/components/StaticPin.d.ts
- lib/commonjs/components/StaticPin.js
- lib/commonjs/helper/applyPanBoundariesToOffset.js
- lib/commonjs/ReactNativeZoomableView.js
- lib/typescript/helper/coordinateConversion.d.ts
- lib/typescript/typings/index.d.ts
- lib/module/helper/coordinateConversion.js
- lib/commonjs/helper/coordinateConversion.js
- lib/typescript/helper/index.d.ts
- lib/module/ReactNativeZoomableView.js
- lib/typescript/ReactNativeZoomableView.d.ts
- lib/commonjs/debugHelper/index.js
- lib/commonjs/components/index.js
- lib/typescript/animations/index.d.ts
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@coderabbitai review |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
package.json (1)
25-29: DRY up build script wiring.
prepublishOnlyandbuildrun the same command. Prefer delegatingprepublishOnlytoyarn buildso build logic has a single source of truth.Suggested cleanup
- "prepublishOnly": "bob build", + "prepublishOnly": "yarn build", ... - "build": "bob build", + "build": "bob build",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 25 - 29, The package.json duplicates build commands; change the "prepublishOnly" script to delegate to the "build" script instead of repeating the bob build invocation: update the "prepublishOnly" value to run "yarn build" (or "npm run build") so the canonical build command remains the "build" script and all publish-time builds use that single source of truth; ensure the "build" script remains set to "bob build" and adjust any CI/publish docs if they referenced the old command.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@package.json`:
- Around line 25-29: The package.json duplicates build commands; change the
"prepublishOnly" script to delegate to the "build" script instead of repeating
the bob build invocation: update the "prepublishOnly" value to run "yarn build"
(or "npm run build") so the canonical build command remains the "build" script
and all publish-time builds use that single source of truth; ensure the "build"
script remains set to "bob build" and adjust any CI/publish docs if they
referenced the old command.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 520da2be-3066-4795-9a14-7452ebea6cdd
⛔ Files ignored due to path filters (26)
lib/commonjs/ReactNativeZoomableView.js.mapis excluded by!**/*.maplib/commonjs/animations/index.js.mapis excluded by!**/*.maplib/commonjs/assets/pin.pngis excluded by!**/*.png,!**/*.pnglib/commonjs/components/AnimatedTouchFeedback.js.mapis excluded by!**/*.maplib/commonjs/components/StaticPin.js.mapis excluded by!**/*.maplib/commonjs/components/index.js.mapis excluded by!**/*.maplib/commonjs/debugHelper/index.js.mapis excluded by!**/*.maplib/commonjs/helper/applyPanBoundariesToOffset.js.mapis excluded by!**/*.maplib/commonjs/helper/calcNewScaledOffsetForZoomCentering.js.mapis excluded by!**/*.maplib/commonjs/helper/coordinateConversion.js.mapis excluded by!**/*.maplib/commonjs/helper/index.js.mapis excluded by!**/*.maplib/commonjs/index.js.mapis excluded by!**/*.maplib/commonjs/typings/index.js.mapis excluded by!**/*.maplib/module/ReactNativeZoomableView.js.mapis excluded by!**/*.maplib/module/animations/index.js.mapis excluded by!**/*.maplib/module/assets/pin.pngis excluded by!**/*.png,!**/*.pnglib/module/components/AnimatedTouchFeedback.js.mapis excluded by!**/*.maplib/module/components/StaticPin.js.mapis excluded by!**/*.maplib/module/components/index.js.mapis excluded by!**/*.maplib/module/debugHelper/index.js.mapis excluded by!**/*.maplib/module/helper/applyPanBoundariesToOffset.js.mapis excluded by!**/*.maplib/module/helper/calcNewScaledOffsetForZoomCentering.js.mapis excluded by!**/*.maplib/module/helper/coordinateConversion.js.mapis excluded by!**/*.maplib/module/helper/index.js.mapis excluded by!**/*.maplib/module/index.js.mapis excluded by!**/*.maplib/module/typings/index.js.mapis excluded by!**/*.map
📒 Files selected for processing (42)
.github/workflows/lint.yml.github/workflows/release.yml.gitignoreCONTRIBUTING.mdlib/commonjs/ReactNativeZoomableView.jslib/commonjs/animations/index.jslib/commonjs/components/AnimatedTouchFeedback.jslib/commonjs/components/StaticPin.jslib/commonjs/components/index.jslib/commonjs/debugHelper/index.jslib/commonjs/helper/applyPanBoundariesToOffset.jslib/commonjs/helper/calcNewScaledOffsetForZoomCentering.jslib/commonjs/helper/coordinateConversion.jslib/commonjs/helper/index.jslib/commonjs/index.jslib/commonjs/typings/index.jslib/module/ReactNativeZoomableView.jslib/module/animations/index.jslib/module/components/AnimatedTouchFeedback.jslib/module/components/StaticPin.jslib/module/components/index.jslib/module/debugHelper/index.jslib/module/helper/applyPanBoundariesToOffset.jslib/module/helper/calcNewScaledOffsetForZoomCentering.jslib/module/helper/coordinateConversion.jslib/module/helper/index.jslib/module/index.jslib/module/typings/index.jslib/typescript/ReactNativeZoomableView.d.tslib/typescript/__tests__/index.test.d.tslib/typescript/animations/index.d.tslib/typescript/components/AnimatedTouchFeedback.d.tslib/typescript/components/StaticPin.d.tslib/typescript/components/index.d.tslib/typescript/debugHelper/index.d.tslib/typescript/helper/applyPanBoundariesToOffset.d.tslib/typescript/helper/calcNewScaledOffsetForZoomCentering.d.tslib/typescript/helper/coordinateConversion.d.tslib/typescript/helper/index.d.tslib/typescript/index.d.tslib/typescript/typings/index.d.tspackage.json
💤 Files with no reviewable changes (36)
- lib/module/components/index.js
- lib/commonjs/typings/index.js
- lib/commonjs/components/index.js
- lib/module/typings/index.js
- lib/typescript/components/index.d.ts
- lib/typescript/index.d.ts
- lib/typescript/components/AnimatedTouchFeedback.d.ts
- lib/module/index.js
- lib/typescript/helper/calcNewScaledOffsetForZoomCentering.d.ts
- lib/typescript/helper/applyPanBoundariesToOffset.d.ts
- lib/module/animations/index.js
- lib/module/helper/calcNewScaledOffsetForZoomCentering.js
- lib/commonjs/index.js
- lib/typescript/animations/index.d.ts
- lib/commonjs/animations/index.js
- lib/module/helper/applyPanBoundariesToOffset.js
- lib/commonjs/helper/index.js
- lib/typescript/helper/index.d.ts
- lib/module/components/AnimatedTouchFeedback.js
- lib/commonjs/helper/applyPanBoundariesToOffset.js
- lib/module/debugHelper/index.js
- lib/commonjs/helper/calcNewScaledOffsetForZoomCentering.js
- lib/commonjs/components/StaticPin.js
- lib/commonjs/helper/coordinateConversion.js
- lib/commonjs/debugHelper/index.js
- lib/commonjs/components/AnimatedTouchFeedback.js
- lib/typescript/components/StaticPin.d.ts
- lib/typescript/debugHelper/index.d.ts
- lib/typescript/helper/coordinateConversion.d.ts
- lib/module/helper/index.js
- lib/typescript/typings/index.d.ts
- lib/module/helper/coordinateConversion.js
- lib/module/components/StaticPin.js
- lib/commonjs/ReactNativeZoomableView.js
- lib/module/ReactNativeZoomableView.js
- lib/typescript/ReactNativeZoomableView.d.ts
…truth Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@claude review |
|
🤖 Agent: @coderabbitai Acknowledged — OIDC trusted publishing is a good follow-up once the pipeline is proven and the npm package is configured for trusted publishing on npmjs.com. Deferring to a future PR. |
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
|
|
@claude review |
There was a problem hiding this comment.
LGTM — all previously identified blocking issues have been resolved.
Extended reasoning...
Overview
This PR moves the build step out of the pre-commit hook and into a dedicated GitHub Actions release workflow. It adds .github/workflows/release.yml, updates .github/workflows/lint.yml, adds /lib/ to .gitignore, renames prepare to prepublishOnly in package.json, and updates CONTRIBUTING.md documentation.
Security risks
No security issues remain. The NPM_TOKEN secret is correctly wired via NODE_AUTH_TOKEN on the publish step, and id-token: write is scoped only to the release job for SLSA provenance attestation — not as a substitute for npm registry auth. Scoped package public access is correctly set with --access public.
Level of scrutiny
This is a CI/CD configuration change affecting only the release pipeline and build tooling, not production library code. The changes are straightforward and all critical correctness issues identified in earlier review rounds have been addressed via commits 06d01b0, bb5d490, aab9e88, b804b68, and f178141.
Other factors
The only remaining item is a minor nit: both the explicit Build step in release.yml and the prepublishOnly lifecycle hook run yarn build during npm publish, causing a redundant build. This wastes a small amount of CI time but does not break correctness — the second build is idempotent. It can be addressed in a follow-up by either removing the explicit Build step (relying solely on prepublishOnly) or passing --ignore-scripts to npm publish (relying solely on the explicit step). This nit is not a blocker.
prepublishOnly already runs `yarn build` before npm publish, so the explicit Build step caused the build to execute twice per release. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add `if: !github.event.release.prerelease` guard to the publish job so pre-releases (e.g. v3.0.0-beta.1) don't get tagged as `latest` on npm. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
LGTM — all functional issues resolved across multiple review rounds.
Extended reasoning...
Overview
PR #158 moves the build step out of the CI lint workflow and into the release workflow, adds npm publishing via GitHub Actions, and cleans up related configuration in package.json, .gitignore, and CONTRIBUTING.md. The changed files are .github/workflows/release.yml, .github/workflows/lint.yml, package.json, .gitignore, and CONTRIBUTING.md (plus committed lib/ artifacts being removed from tracking).
Security risks
The release workflow handles npm publishing with a scoped token (NPM_TOKEN). After several review rounds, the workflow now correctly: (1) authenticates via NODE_AUTH_TOKEN, (2) uses --provenance for signed attestations backed by the id-token: write OIDC permission, (3) guards against pre-releases polluting the latest dist-tag with if: ${{ \!github.event.release.prerelease }}, and (4) publishes with --access public for the scoped package. No security regressions are present.
Level of scrutiny
This is a CI/CD configuration change with real publishing implications — it directly controls when and how the npm package is released. It warranted careful review, which it received. The workflow is now minimal and correct. The only remaining item is a minor documentation gap in CONTRIBUTING.md (not mentioning that pre-releases skip npm publish), captured in the inline comment.
Other factors
Seven prior inline bugs were raised and all have been resolved. The PR has gone through multiple commit iterations addressing each concern. The remaining nit is documentation-only and does not affect correctness or security.
Explain that GitHub pre-releases intentionally skip npm publish and how to publish a pre-release manually with a dist-tag. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
LGTM — all previously raised issues have been resolved.
Extended reasoning...
Overview
This PR moves the build step out of git history and into CI/CD: lib/ is removed from version control and git tracking, a new release.yml workflow builds and publishes to npm on GitHub Release events, and lint.yml gains an explicit build step for PR validation.
Security risks
The publish workflow uses secrets.NPM_TOKEN via NODE_AUTH_TOKEN for registry auth, and id-token: write permission solely for SLSA provenance attestations (--provenance flag). No secrets are exposed beyond what is required. Scoped package is published with --access public as needed for @openspacelabs/*. No security concerns.
Level of scrutiny
CI/CD configuration changes warrant careful review, but the scope here is narrow: two workflow files, package.json lifecycle hooks, .gitignore, and CONTRIBUTING.md. The changes are mechanical and follow well-established patterns for npm publishing via GitHub Actions.
Other factors
Multiple rounds of review caught and resolved: missing NODE_AUTH_TOKEN, incorrect OIDC claims, prepare vs prepublishOnly lifecycle hook, release-it.npm.publish misconfiguration, missing --provenance flag, duplicate build execution, absent pre-release guard, and undocumented pre-release behavior. All issues have been marked resolved by the author. The bug hunting system found no new issues. The PR is ready to merge.
PR #158 removed checked-in lib/ and added it to .gitignore. These remaining tracked files are now redundant.
PR #158 removed checked-in lib/ and added it to .gitignore. These remaining tracked files are now redundant.
PR #158 removed checked-in lib/ and added it to .gitignore. These remaining tracked files are now redundant.
PR #158 removed checked-in lib/ and added it to .gitignore. These remaining tracked files are now redundant.
PR #158 removed checked-in lib/ and added it to .gitignore. These remaining tracked files are now redundant.
PR #158 removed checked-in lib/ and added it to .gitignore. These remaining tracked files are now redundant.
2 participants
Summary
Changes
bob buildfrom pre-commit hookRelease Process
yarn releaselocally (creates tag + GitHub release)Requires
NPM_TOKENsecret to be set in GitHub repo settings.Note
Implements CI/CD for builds and releases, and removes built output from the repo.
.github/workflows/lint.ymlto run onpush(master) andpull_request; sets up Node 16 with yarn cache; installs deps; runs TypeScript, ESLint, andreact-native-builder-bobbuild.github/workflows/release.ymlto build andnpm publishon GitHub Release creation (usesNPM_TOKEN).github/release.ymlwith label-based release notes categories/exclusionslib/to.gitignoreand removes committedlib/commonjs/*build artifactsCONTRIBUTING.mdandREADME.mdwith new build, release, and changelog processWritten by Cursor Bugbot for commit 205b2df. Configure here.
Summary by CodeRabbit
New Features
Chores
Documentation
Breaking Changes