Closed
Conversation
|
@gowthamkishore3799 is attempting to deploy a commit to the Inbox Zero Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
## Walkthrough
This set of changes updates environment variable values, workflow configuration, and import orderings across several files. The placeholder value `'dummy-sanity-project-id-for-build'` for the `NEXT_PUBLIC_SANITY_PROJECT_ID` environment variable is replaced with `"project123"` in both the Dockerfile and multiple TypeScript files, altering the logic that determines when to skip Sanity fetch operations during builds. Additionally, the Docker image publishing workflow is updated to use a fixed username `"github-actions"` for registry login and adds a `run-name` for clarity. The docker-compose service is modified to always pull the latest image from the registry. Import statements in several files are reordered for consistency, but these changes do not affect functionality.
## Changes
| File(s) | Change Summary |
|-----------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------|
| `.github/workflows/publish-docker.yml` | Added `run-name` to workflow; changed login username to fixed `"github-actions"`; registry remains `ghcr.io`. |
| `docker/Dockerfile.prod` | Replaced dummy `NEXT_PUBLIC_SANITY_PROJECT_ID` values with `"project123"` in ENV declarations; trimmed CMD line whitespace. |
| `docker-compose.yml` | Added `image: ghcr.io/elie222/inbox-zero:latest` and `pull_policy: always` to `web` service for always pulling latest image. |
| `apps/web/app/blog/page.tsx`, `apps/web/app/blog/post/[slug]/page.tsx`, `apps/web/app/sitemap.ts` | Reordered imports; changed environment variable check from `'dummy-sanity-project-id-for-build'` to `"project123"` to control skipping Sanity fetch logic. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant NextApp
participant SanityAPI
User->>NextApp: Request blog content or sitemap
alt NEXT_PUBLIC_SANITY_PROJECT_ID == "project123"
NextApp-->>User: Return empty results (skip Sanity fetch)
else
NextApp->>SanityAPI: Fetch blog data
SanityAPI-->>NextApp: Return posts
NextApp-->>User: Serve content
endPossibly related PRs
Poem
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/publish-docker.yml(2 hunks)apps/web/app/blog/page.tsx(2 hunks)apps/web/app/blog/post/[slug]/page.tsx(1 hunks)apps/web/app/sitemap.ts(1 hunks)docker/Dockerfile.prod(3 hunks)
🔇 Additional comments (11)
docker/Dockerfile.prod (3)
50-50: Consistent project ID update to fix Docker build issue.This change aligns with the PR objective to fix Docker image build failures by replacing hyphenated IDs with a plain string format.
76-76: Consistent project ID update at second declaration point.The environment variable is correctly updated with the same value as on line 50, ensuring consistency throughout different build stages.
87-87: Minor whitespace cleanup.Removed trailing whitespace while keeping the command functionality intact.
apps/web/app/sitemap.ts (2)
3-4: Import reordering.Imports were reordered for better organization but functionality remains unchanged.
8-8: Consistent project ID check for skipping Sanity fetch.This change properly aligns with the Dockerfile updates, ensuring the application code consistently checks for the new non-hyphenated project ID.
apps/web/app/blog/post/[slug]/page.tsx (2)
3-8: Import reordering.Imports were reordered for better organization but functionality remains unchanged.
13-13: Consistent project ID check for skipping Sanity fetch.This change properly aligns with the Dockerfile updates, ensuring the static parameter generation consistently checks for the new non-hyphenated project ID.
.github/workflows/publish-docker.yml (1)
2-2: Added descriptive run-name for the workflow.Adding a run-name improves workflow clarity in the GitHub Actions UI.
apps/web/app/blog/page.tsx (3)
1-7: Import organization improves readabilityThe reordering of imports follows a logical pattern with types first, then components, and finally framework imports. This organization enhances code readability.
211-211: Environment variable check updated correctlyThe change from using a hyphenated placeholder to
"project123"correctly addresses the Docker build issue mentioned in the PR objectives. Removing hyphens from the ID format aligns with the expected object ID format in the Docker build environment.
208-213:Details
✅ Verification successful
Verify consistent environment variable usage across the codebase
The environment variable check has been modified to use the new format without hyphens. According to the PR summary, this change should also exist in other files like
apps/web/app/blog/post/[slug]/page.tsxandapps/web/app/sitemap.ts.
🏁 Script executed:
#!/bin/bash # Verify consistent usage of the updated environment variable value echo "Checking usage of NEXT_PUBLIC_SANITY_PROJECT_ID across the codebase..." rg "NEXT_PUBLIC_SANITY_PROJECT_ID.*project123" --type=ts --type=tsx echo "Checking if there are any remaining instances of the old dummy ID..." rg "dummy-sanity-project-id-for-build" --type=ts --type=tsxLength of output: 483
🏁 Script executed:
#!/bin/bash # Retry searching with explicit glob patterns for TS/TSX files echo "Checking usage of NEXT_PUBLIC_SANITY_PROJECT_ID across the codebase..." rg "NEXT_PUBLIC_SANITY_PROJECT_ID" -g "*.ts" -g "*.tsx" echo echo "Checking for hardcoded project123 values..." rg "project123" -g "*.ts" -g "*.tsx" echo echo "Checking if there are any remaining instances of the old dummy ID..." rg "dummy-sanity-project-id-for-build" -g "*.ts" -g "*.tsx"Length of output: 1391
Environment variable usage is consistent across the codebase
- Verified that
NEXT_PUBLIC_SANITY_PROJECT_IDis checked in:
- apps/web/app/blog/page.tsx
- apps/web/app/blog/post/[slug]/page.tsx
- apps/web/app/sitemap.ts
- apps/web/sanity.cli.ts
- apps/web/sanity/env.ts
- No remaining occurrences of the old dummy ID (
dummy-sanity-project-id-for-build) were found.
All instances have been updated as expected. 🎉
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docker-compose.yml (1)
41-42: Verify Compose support forpull_policyand refine image/build strategy.The
pull_policy: alwaysoption is only recognized in Docker Compose spec v3.9+ (Docker Compose CLI v2). Please confirm your CI/CD and local environments support this key; otherwise it will be ignored or error out. Also, you’re mixing a published image (ghcr.io/elie222/inbox-zero:latest) with a localbuildcontext:
- If your goal is to always pull the pre-built image, consider removing the
build:block entirely.- If you need to build locally and tag it, then remove
pull_policy(or move this into a dedicated production Compose file) to avoid unwanted pulls during development.Finally, using
latestcan lead to non-reproducible deployments—pinning to a specific version tag or digest is recommended for stability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/publish-docker.yml(2 hunks)docker-compose.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/publish-docker.yml
Contributor
Author
|
@elie222 do check this out once available, it should fix the error which the current version is facing Have signed the CLA as well |
Contributor
Author
There was a problem hiding this comment.
its just lint changes, no code changes
| import imageUrlBuilder from "@sanity/image-url"; | ||
| import { Post } from "@/app/blog/post/[slug]/Post"; | ||
| import type { Post as PostType } from "@/app/blog/types"; | ||
| import { client } from "@/sanity/lib/client"; |
Contributor
Author
There was a problem hiding this comment.
Its just lint changes, no code changes
Contributor
Author
|
Since the pr is removed, closing this pull request |
Contributor
Author
|
Btw, i could see the docker image build is removed? is it open for me to pickup or anyones working on it I could revert the changes which caused the production build to fail, and have pipeline which doesnt impact the production build so semantic versioning and docker image build is gracefully handled |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixing the Current Docker Image Build Failure
The Docker image build is currently failing due to an invalid object ID format. The build process expects the object ID to be a plain string without any hyphens (-), but the current value includes hyphens, causing the failure.
Summary by CodeRabbit