Skip to content

WIP: improve stacks state and local development#40

Draft
Olaleye-Blessing wants to merge 3 commits intodevfrom
improve-devvv
Draft

WIP: improve stacks state and local development#40
Olaleye-Blessing wants to merge 3 commits intodevfrom
improve-devvv

Conversation

@Olaleye-Blessing
Copy link
Copy Markdown
Collaborator

No description provided.

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
app-stacks Error Error Mar 9, 2026 5:20am

@netlify
Copy link
Copy Markdown

netlify Bot commented Mar 9, 2026

Deploy Preview for app-stacks ready!

Name Link
🔨 Latest commit 6bd7217
🔍 Latest deploy log https://app.netlify.com/projects/app-stacks/deploys/69c094a933ca5d0008307fa5
😎 Deploy Preview https://deploy-preview-40--app-stacks.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@tbsoc
Copy link
Copy Markdown
Member

tbsoc commented Mar 9, 2026

🤖 AI Code Review

PR Review: WIP: improve stacks state and local development

1. Summary

This PR does three things:

  • Updates UI package: Bumps @breadcoop/ui from 1.0.25 to 1.0.26
  • Updates git submodule: Advances saving-circles submodule to commit c231e63
  • Adds local dev command: New make start-local target that clears build artifacts, deploys contracts, and starts the dev server
  • Dependency maintenance: Updates 15+ packages including Privy, RainbowKit, Wagmi, Viem, and Tailwind to latest patch/minor versions

2. Code Quality

Good:

  • Pinning @wagmi/core to exact version (2.22.1 instead of 2.x.x) is a solid improvement for reproducible builds
  • The start-local target provides a clean one-command setup for fresh local development

Concerns:

  • Redundant comments in the makefile: Lines 122-124 have nearly identical warnings about restarting anvil. Consolidate to one clear instruction.
  • Destructive cleanup: rm -rf contracts/broadcast contracts/cache contracts/out permanently deletes deployment artifacts. The broadcast folder contains deployment records that might be needed for debugging or verification.
  • No error handling: If make deploy fails, the command continues to pnpm run dev which could run against stale or broken contract state.

Architecture:

  • Submodule updates mixed with dependency bumps and new features make this PR do too many things. Consider splitting.

3. Security

Low risk but worth noting:

  • Binary blobs: The .tgz files are opaque binaries. Ensure the new UI package version has been reviewed/tested before merging.
  • Submodule trust: The saving-circles submodule moved from ed1f7b8 to c231e63. Verify this commit is from a trusted source and doesn't introduce malicious contract code.
  • Dependency updates: Most are minor/patch bumps within established packages (Privy, RainbowKit, Viem). No red flags in the lockfile diff.

4. Suggestions

Makefile improvements:

# Replace the redundant comments with:
start-local: ## Fresh local dev setup (WARNING: destroys broadcast/ cache/ out/)
	@echo "Clearing contract build artifacts..."
	rm -rf contracts/broadcast contracts/cache contracts/out
	$(MAKE) deploy && pnpm run dev || (echo "Deploy failed. Fix errors before running dev." && exit 1)

Additional recommendations:

  1. Preserve broadcast selectively: Consider if you really need to delete broadcast — it contains deployment addresses and transaction receipts. Maybe only clear cache and out for a "clean build" while keeping broadcast for reference.

  2. Add PR description: The PR title says "improve stacks state" but there's no description. Explain what state issues this fixes and what the submodule update brings.

  3. Separate concerns: The UI package update and dependency bumps could be their own PR. The start-local command and submodule update feel like distinct changes.

  4. Verify lockfile integrity: The pnpm-lock.yaml diff is large. Run pnpm install --frozen-lockfile in CI to ensure it stays valid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants