Skip to content

[codex] Add Shakapacker brand assets - #1135

Merged
justin808 merged 5 commits into
mainfrom
jg-conductor/shakapacker-icons-issue
May 24, 2026
Merged

[codex] Add Shakapacker brand assets#1135
justin808 merged 5 commits into
mainfrom
jg-conductor/shakapacker-icons-issue

Conversation

@justin808

@justin808 justin808 commented May 24, 2026

Copy link
Copy Markdown
Member

Summary

Adds the Shakapacker folded-S brand asset set, including PNG icon sizes, SVG mark/tile assets, and light/dark lockups. Features the light/dark brand lockup near the top of README.md, making the repo landing page show the new branding immediately. Adds root apple-touch-icon.png, favicon.png, favicon.svg, and assets/icon.png so Conductor and other development tools can discover the project icon. Follow-up website rollout is tracked in shakacode/shakapacker.com#20.

Pull Request checklist

  • Add/update test to cover these changes
  • Update documentation
  • Update CHANGELOG file

Other Information

Validation:

  • all brand assets match source directory
  • Conductor first match: apple-touch-icon.png
  • npm pack --dry-run --json
  • gemspec file check reported no added icon assets
  • yarn prettier --check README.md assets/brand/README.md

Note

Low Risk
Low risk: documentation-only change plus new static SVG brand assets; no runtime code paths or security-sensitive logic are modified.

Overview
Adds a centered Shakapacker logo lockup image at the top of README.md to brand the repo landing page.

Introduces canonical brand asset sources under assets/brand/ (icon tile, mark, and light/dark lockups) plus a root favicon.svg, with a small assets/brand/README.md describing how to keep discovery copies in sync for tooling.

Reviewed by Cursor Bugbot for commit f93f7a5. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Documentation
    • Added Shakapacker logo to the main README
    • Created brand assets documentation detailing canonical icons, lockup assets, and synchronization guidance

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 19b48dad-8dce-485b-b09d-1cd26d43e289

📥 Commits

Reviewing files that changed from the base of the PR and between 75515af and f93f7a5.

⛔ Files ignored due to path filters (16)
  • apple-touch-icon.png is excluded by !**/*.png
  • assets/brand/icon-1024.png is excluded by !**/*.png
  • assets/brand/icon-128.png is excluded by !**/*.png
  • assets/brand/icon-16.png is excluded by !**/*.png
  • assets/brand/icon-256.png is excluded by !**/*.png
  • assets/brand/icon-32.png is excluded by !**/*.png
  • assets/brand/icon-512.png is excluded by !**/*.png
  • assets/brand/icon-64.png is excluded by !**/*.png
  • assets/brand/icon-tile.svg is excluded by !**/*.svg
  • assets/brand/lockup-dark.svg is excluded by !**/*.svg
  • assets/brand/lockup-light.png is excluded by !**/*.png
  • assets/brand/lockup-light.svg is excluded by !**/*.svg
  • assets/brand/mark-transparent.svg is excluded by !**/*.svg
  • assets/icon.png is excluded by !**/*.png
  • favicon.png is excluded by !**/*.png
  • favicon.svg is excluded by !**/*.svg
📒 Files selected for processing (2)
  • README.md
  • assets/brand/README.md

Walkthrough

This PR adds branding and documentation to the Shakapacker repository. The root README now displays a centered logo header linking to shakapacker.com. A new README in the assets/brand directory documents the canonical icon and lockup assets, their relationship to discovery files, synchronization requirements, and raster/SVG variant guidance.

Changes

Documentation and Branding Assets

Layer / File(s) Summary
Root README branding header
README.md
Adds a centered HTML header block containing a linked Shakapacker logo image at the top of the README.
Brand assets documentation
assets/brand/README.md
Documents the purpose of brand assets, canonical icon and lockup locations, the relationship between root discovery files and source assets, synchronization requirements when updating icons, and clarifies light/dark raster vs SVG guidance.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A logo graces the README crown,
Brand assets documented in the town,
Icons synced from source to sight,
SVG dark and raster light,
Shakapacker's visual identity bright! ✨

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jg-conductor/shakapacker-icons-issue

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude

claude Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Code Review

Overview: Adds the Shakapacker folded-S brand asset set — PNG icons at multiple sizes, SVG mark/tile/lockup variants, and root-level favicon/icon files for Conductor and other dev-tool discovery.

Overall this is a clean, low-risk PR. The gem and npm packaging are both safe: the gemspec explicitly lists paths that exclude assets/, and package.json's files field only includes package/ and one config file.


Issues

Missing trailing newlines (critical — project linting rule)

All five SVG files end without a trailing newline, which is explicitly forbidden by the project's linting rules (CLAUDE.md: "ALWAYS end all files with a trailing newline character. This is required by the project's linting rules."):

  • favicon.svg
  • assets/brand/icon-tile.svg
  • assets/brand/mark-transparent.svg
  • assets/brand/lockup-dark.svg
  • assets/brand/lockup-light.svg

Each file ends with </svg> with no newline. Fix: add a newline after the closing tag.

Trailing whitespace in lockup SVGs

Both lockup-dark.svg and lockup-light.svg have a blank line with trailing spaces on line 9 (two spaces after the inner <rect>). Should be a clean empty line or removed entirely.


Minor observations

  • assets/icon.png is a byte-for-byte copy of assets/brand/icon-1024.png. The README explains this is an intentional fallback for tools that look in assets/, which is fine — worth noting so future contributors know not to diverge them unintentionally.
  • lockup-light.png (binary) exists but there is no lockup-dark.png binary. Only the SVG dark variant is present. No impact now, but the asymmetry may confuse future contributors.
  • rx="0" on the background rects in the lockup SVGs is redundant (0 is the SVG default) — cosmetic, no action needed.

Comment thread favicon.svg Outdated
<path d="M300 276 H704 L604 394 H452 Z" fill="#FF5A4C" opacity="0.32"/>
<path d="M354 626 H506 L594 752 H224 Z" fill="#E05D00" opacity="0.20"/>
</g>
</svg> No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing trailing newline — the project requires all files to end with a newline character.

Suggested change
</svg>
</svg>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b9fb4cc — trailing newline added.

Comment thread assets/brand/icon-tile.svg Outdated
<path d="M300 276 H704 L604 394 H452 Z" fill="#FF5A4C" opacity="0.32"/>
<path d="M354 626 H506 L594 752 H224 Z" fill="#E05D00" opacity="0.20"/>
</g>
</svg> No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing trailing newline — the project requires all files to end with a newline character.

Suggested change
</svg>
</svg>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b9fb4cc — trailing newline added.

Comment thread assets/brand/mark-transparent.svg Outdated
<path d="M300 276 H704 L604 394 H452 Z" fill="#FF5A4C" opacity="0.32"/>
<path d="M354 626 H506 L594 752 H224 Z" fill="#E05D00" opacity="0.20"/>
</g>
</svg> No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing trailing newline — the project requires all files to end with a newline character.

Suggested change
</svg>
</svg>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b9fb4cc — trailing newline added.

Comment thread assets/brand/lockup-dark.svg Outdated
</g>
<text x="410" y="180" font-family="Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, Segoe UI, Arial, sans-serif" font-weight="800" font-size="92" fill="#E6EDF3">Shakapacker</text>
<text x="414" y="252" font-family="Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, Segoe UI, Arial, sans-serif" font-weight="500" font-size="40" fill="#8B949E">Rails asset bundling with modern build systems</text>
</svg> No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing trailing newline — the project requires all files to end with a newline character.

Suggested change
</svg>
</svg>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b9fb4cc — trailing newline added.

Comment thread assets/brand/lockup-dark.svg Outdated
<rect x="66" y="66" width="892" height="892" rx="176" fill="none" stroke="#293544" stroke-width="20"/>

<g stroke-linejoin="round">
<path d="M300 276 H704 L604 394 H452 L564 492 H408 L276 394 Z" fill="#E43D39"/>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line contains trailing whitespace (two spaces). Should be an empty line.

Suggested change
<path d="M300 276 H704 L604 394 H452 L564 492 H408 L276 394 Z" fill="#E43D39"/>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b9fb4cc — trailing whitespace removed.

Comment thread assets/brand/lockup-light.svg Outdated
</g>
<text x="410" y="180" font-family="Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, Segoe UI, Arial, sans-serif" font-weight="800" font-size="92" fill="#24292F">Shakapacker</text>
<text x="414" y="252" font-family="Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, Segoe UI, Arial, sans-serif" font-weight="500" font-size="40" fill="#8B949E">Rails asset bundling with modern build systems</text>
</svg> No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing trailing newline — the project requires all files to end with a newline character.

Suggested change
</svg>
</svg>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b9fb4cc — trailing newline added.

Comment thread assets/brand/lockup-light.svg Outdated
<rect x="66" y="66" width="892" height="892" rx="176" fill="none" stroke="#293544" stroke-width="20"/>

<g stroke-linejoin="round">
<path d="M300 276 H704 L604 394 H452 L564 492 H408 L276 394 Z" fill="#E43D39"/>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line contains trailing whitespace (two spaces). Should be an empty line.

Suggested change
<path d="M300 276 H704 L604 394 H452 L564 492 H408 L276 394 Z" fill="#E43D39"/>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b9fb4cc — trailing whitespace removed.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Registry README breaks lockup images
    • Replaced the registry README lockup image paths with absolute raw GitHub URLs that resolve outside the repository checkout.

Create PR

Or push these changes by commenting:

@cursor push b59ccfa947
Preview (b59ccfa947)
diff --git a/README.md b/README.md
--- a/README.md
+++ b/README.md
@@ -3,9 +3,9 @@
 <p align="center">
   <a href="https://shakapacker.com">
     <picture>
-      <source media="(prefers-color-scheme: dark)" srcset="./assets/brand/lockup-dark.svg">
-      <source media="(prefers-color-scheme: light)" srcset="./assets/brand/lockup-light.svg">
-      <img src="./assets/brand/lockup-light.svg" alt="Shakapacker: Rails asset bundling with modern build systems" width="760">
+      <source media="(prefers-color-scheme: dark)" srcset="https://raw.githubusercontent.com/shakacode/shakapacker/7290273b23a1351912b47fb4e1303cec2fa8de8f/assets/brand/lockup-dark.svg">
+      <source media="(prefers-color-scheme: light)" srcset="https://raw.githubusercontent.com/shakacode/shakapacker/7290273b23a1351912b47fb4e1303cec2fa8de8f/assets/brand/lockup-light.svg">
+      <img src="https://raw.githubusercontent.com/shakacode/shakapacker/7290273b23a1351912b47fb4e1303cec2fa8de8f/assets/brand/lockup-light.svg" alt="Shakapacker: Rails asset bundling with modern build systems" width="760">
     </picture>
   </a>
 </p>

You can send follow-ups to the cloud agent here.

Comment thread README.md
…ge URLs

- Add trailing newline to favicon.svg, icon-tile.svg, mark-transparent.svg,
  lockup-dark.svg, lockup-light.svg per project linting rule
- Remove trailing whitespace on the blank line in lockup-dark.svg and
  lockup-light.svg
- Switch README brand lockup <picture> sources from relative
  ./assets/brand/ paths to absolute raw.githubusercontent.com URLs so the
  hero images resolve on the npm and RubyGems registry README pages
  (the gemspec and package.json do not ship the assets/ directory)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@justin808

Copy link
Copy Markdown
Member Author

Review feedback addressed in b9fb4cc

# Reviewer Item Resolution
1 @claude favicon.svg missing trailing newline Added
2 @claude assets/brand/icon-tile.svg missing trailing newline Added
3 @claude assets/brand/mark-transparent.svg missing trailing newline Added
4 @claude assets/brand/lockup-dark.svg missing trailing newline Added
5 @claude assets/brand/lockup-dark.svg trailing whitespace on line 7 Removed
6 @claude assets/brand/lockup-light.svg missing trailing newline Added
7 @claude assets/brand/lockup-light.svg trailing whitespace on line 7 Removed
8 Cursor Bugbot (Medium) README lockup images would 404 on npm/RubyGems registry pages because the gem and npm tarball don't ship assets/ Switched to absolute raw.githubusercontent.com/shakacode/shakapacker/main/assets/brand/... URLs (same pattern as the existing sponsor logos lower in the README)

Informational observations from @claude's review (no action taken):

  • assets/icon.png is intentionally a byte-for-byte copy of assets/brand/icon-1024.png — documented in assets/README.md as the fallback path for tools that probe assets/.
  • Only lockup-light.png ships as a binary; lockup-dark is SVG-only. Cosmetic asymmetry, not load-bearing.
  • rx="0" on the lockup background rect is redundant with the SVG default — cosmetic.

@claude

claude Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Code Review: Add Shakapacker brand assets

Overview

This PR adds the Shakapacker folded-S brand asset set (SVGs, PNGs in various sizes, root favicons) and updates the README to show the light/dark lockup image. The intent is clear and the asset set is well-organized.

Issues

Critical: Missing trailing newlines in all SVG files

All five SVG files end without a trailing newline, violating the project's critical linting requirement (CLAUDE.md and .claude/rules/coding-style.md). Files affected:

  • favicon.svg
  • assets/brand/icon-tile.svg
  • assets/brand/lockup-dark.svg
  • assets/brand/lockup-light.svg
  • assets/brand/mark-transparent.svg

README relative paths will break on RubyGems

The <picture> block uses relative paths ./assets/brand/lockup-*.svg. These render on GitHub, but the gemspec only bundles CHANGELOG.md MIT-LICENSE README.md package.json shakapacker.gemspec lib sig — the brand assets are excluded from the gem. On rubygems.org the images will appear broken. Consider switching to absolute GitHub raw content URLs so the README renders everywhere.

Minor

  • Redundant rx="0" on the outer <rect> in both lockup SVGs — rx defaults to 0, the attribute is a no-op.
  • Inconsistent indentation inside SVGs: the <g stroke-linejoin="round"> block uses 8-space indentation while surrounding elements use 0- or 2-space. A prettier pass would normalize this.
  • Asymmetric PNG set: lockup-light.png is present but there is no lockup-dark.png. If the PNG is unused (only SVGs are referenced from the README), consider removing it to avoid confusion.

Positives

  • Light/dark <picture> fallback in README is implemented correctly with proper source ordering.
  • All SVG files include <title> and ARIA attributes — good accessibility.
  • Gemspec and package.json files field correctly exclude the new assets from published packages.
  • assets/brand/README.md clearly documents the purpose of each file.

<path d="M300 276 H704 L604 394 H452 Z" fill="#FF5A4C" opacity="0.32"/>
<path d="M354 626 H506 L594 752 H224 Z" fill="#E05D00" opacity="0.20"/>
</g>
</svg>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing trailing newline — the project requires all files to end with a newline character (enforced by linting rules). Please add a newline after </svg>.

</g>
<text x="410" y="180" font-family="Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, Segoe UI, Arial, sans-serif" font-weight="800" font-size="92" fill="#E6EDF3">Shakapacker</text>
<text x="414" y="252" font-family="Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, Segoe UI, Arial, sans-serif" font-weight="500" font-size="40" fill="#8B949E">Rails asset bundling with modern build systems</text>
</svg>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing trailing newline — same issue as the other SVG files. Please append a newline after </svg>.

Comment thread assets/brand/lockup-dark.svg Outdated
@@ -0,0 +1,17 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1600 420" role="img" aria-label="Shakapacker logo lockup">
<title>Shakapacker lockup</title>
<rect width="1600" height="420" rx="0" fill="#0D1117"/>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rx="0" is the SVG default and can be removed — it adds visual noise without effect. Same applies to the equivalent line in lockup-light.svg.

Suggested change
<rect width="1600" height="420" rx="0" fill="#0D1117"/>
<rect width="1600" height="420" fill="#0D1117"/>

</g>
<text x="410" y="180" font-family="Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, Segoe UI, Arial, sans-serif" font-weight="800" font-size="92" fill="#24292F">Shakapacker</text>
<text x="414" y="252" font-family="Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, Segoe UI, Arial, sans-serif" font-weight="500" font-size="40" fill="#8B949E">Rails asset bundling with modern build systems</text>
</svg>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing trailing newline — please append a newline after </svg>.

<path d="M300 276 H704 L604 394 H452 Z" fill="#FF5A4C" opacity="0.32"/>
<path d="M354 626 H506 L594 752 H224 Z" fill="#E05D00" opacity="0.20"/>
</g>
</svg>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing trailing newline — please append a newline after </svg>.

Comment thread favicon.svg
<path d="M300 276 H704 L604 394 H452 Z" fill="#FF5A4C" opacity="0.32"/>
<path d="M354 626 H506 L594 752 H224 Z" fill="#E05D00" opacity="0.20"/>
</g>
</svg>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing trailing newline — please append a newline after </svg>.

@claude

claude Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Review of PR 1135: Add Shakapacker Brand Assets

Overall: Low-risk, asset-only change that correctly avoids gem bloat. A few items worth addressing.

Gemspec / Packaging

The gemspec uses an explicit file list (CHANGELOG.md MIT-LICENSE README.md package.json shakapacker.gemspec lib sig), so none of the new assets/, favicon.*, or apple-touch-icon.png files end up in the published gem. Good.

Duplicate / Copied Files

The brand README correctly documents these as intentional copies for tool discovery, but the repo now carries byte-for-byte duplicates:

  • favicon.svg = assets/brand/icon-tile.svg (identical content, confirmed by hash)
  • favicon.png = assets/brand/icon-32.png (same git object)
  • apple-touch-icon.png = assets/brand/icon-256.png (same git object)
  • assets/icon.png = assets/brand/icon-1024.png (same git object)

Git deduplicates objects internally so the storage cost is negligible, but future manual updates risk the copies drifting out of sync with their canonicals. Consider a note in assets/brand/README.md about how to keep root copies in sync (e.g. a regeneration command or explicit update step).

SVG Indentation Inconsistency

All SVG files mix zero-indent root elements with 8-space-indented path elements inside <g>. Appears to be a copy-paste or code-generation artifact. Cosmetic, but a consistent indent style would be cleaner.

Dark Lockup: Icon Background Blends Into Lockup Background

In lockup-dark.svg, the icon rounded-rect uses fill="#0D1117" — the same colour as the outer lockup background (fill="#0D1117"). The only visual delineation is a subtle stroke="#293544" border. The red/orange mark paths still render, but the rounded-square silhouette disappears. Compare with the light lockup where fill="#0D1117" gives strong contrast against white. If the floating-mark effect is intentional, it is worth noting explicitly.

Minor: Redundant rx="0" Attribute

Both lockup SVGs include rx="0" on the full-bleed background rectangle. This is the SVG default and can be omitted without any visual change.

Asymmetry: lockup-light.png but No lockup-dark.png

A rasterised lockup-light.png is included but there is no lockup-dark.png. If the PNG is only needed as a preview/fallback tool, this is fine — worth noting the intentional omission in assets/brand/README.md to avoid a future contributor adding a dark PNG unilaterally.

README <picture> Tag

The <picture> element with raw.githubusercontent.com/.../main/... URLs is the correct standard approach for GitHub-rendered READMEs. The fallback <img> using the light lockup is a sensible default for non-GitHub renderers.

Comment thread assets/brand/lockup-dark.svg Outdated
@@ -0,0 +1,17 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1600 420" role="img" aria-label="Shakapacker logo lockup">
<title>Shakapacker lockup</title>
<rect width="1600" height="420" rx="0" fill="#0D1117"/>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rx="0" attribute is the SVG default and has no effect — it can be dropped.

Suggested change
<rect width="1600" height="420" rx="0" fill="#0D1117"/>
<rect width="1600" height="420" fill="#0D1117"/>

Comment thread assets/brand/lockup-dark.svg Outdated
<title>Shakapacker lockup</title>
<rect width="1600" height="420" rx="0" fill="#0D1117"/>
<g transform="translate(70,66) scale(0.28)">
<rect x="56" y="56" width="912" height="912" rx="186" fill="#0D1117"/>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The icon rounded-rect fill (#0D1117) matches the outer lockup background (#0D1117 on line 3). This makes the rounded-square silhouette of the icon invisible — only the faint stroke="#293544" border on the next line provides any separation. The red/orange paths still appear, but the icon reads as floating rather than inside a tile.

If the floating-mark effect is intentional, it might be worth a quick note in assets/brand/README.md. If not, consider using a slightly lighter fill (e.g. #161B22) to give the icon tile a visible background against the dark lockup.

Comment thread assets/brand/lockup-light.svg Outdated
@@ -0,0 +1,17 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1600 420" role="img" aria-label="Shakapacker logo lockup">
<title>Shakapacker lockup</title>
<rect width="1600" height="420" rx="0" fill="#FFFFFF"/>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the dark lockup — rx="0" is the default and can be removed.

Suggested change
<rect width="1600" height="420" rx="0" fill="#FFFFFF"/>
<rect width="1600" height="420" fill="#FFFFFF"/>

Comment thread assets/brand/icon-tile.svg Outdated
<rect x="56" y="56" width="912" height="912" rx="186" fill="#0D1117"/>
<rect x="66" y="66" width="892" height="892" rx="176" fill="none" stroke="#293544" stroke-width="20"/>

<g stroke-linejoin="round">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The <g> block has 8-space indentation while the surrounding root elements (<rect>, <title>) have no indentation at all. Looks like a copy-paste artifact. Consider normalising to 2-space indent throughout the file (and favicon.svg, which is identical).

<path d="M354 626 H506 L594 752 H224 Z" fill="#E05D00" opacity="0.20"/>
</g>
</g>
<text x="410" y="180" font-family="Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, Segoe UI, Arial, sans-serif" font-weight="800" font-size="92" fill="#E6EDF3">Shakapacker</text>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor inconsistency: the title text starts at x="410" but the subtitle uses x="414" — a 4px horizontal offset. Same pattern exists in lockup-light.svg. If this is intentional for optical alignment, a comment in assets/brand/README.md would clarify intent; if accidental, the subtitle should also use x="410".

Suggested change
<text x="410" y="180" font-family="Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, Segoe UI, Arial, sans-serif" font-weight="800" font-size="92" fill="#E6EDF3">Shakapacker</text>
<text x="410" y="252" font-family="Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, Segoe UI, Arial, sans-serif" font-weight="500" font-size="40" fill="#8B949E">Rails asset bundling with modern build systems</text>

</g>
</g>
<text x="410" y="180" font-family="Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, Segoe UI, Arial, sans-serif" font-weight="800" font-size="92" fill="#24292F">Shakapacker</text>
<text x="414" y="252" font-family="Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, Segoe UI, Arial, sans-serif" font-weight="500" font-size="40" fill="#8B949E">Rails asset bundling with modern build systems</text>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same 4px x-offset as lockup-dark.svg — title at x="410", subtitle at x="414". Should be aligned or the intent documented.

Suggested change
<text x="414" y="252" font-family="Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, Segoe UI, Arial, sans-serif" font-weight="500" font-size="40" fill="#8B949E">Rails asset bundling with modern build systems</text>
<text x="410" y="252" font-family="Inter, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, Segoe UI, Arial, sans-serif" font-weight="500" font-size="40" fill="#8B949E">Rails asset bundling with modern build systems</text>

@claude

claude Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Code Review

Summary

Clean, low-risk PR that adds the Shakapacker brand asset set and updates the README header. No runtime code paths are affected.


Packaging safety ✅

Verified both distribution channels are safe:

  • Ruby gem: gemspec uses an explicit file list (CHANGELOG.md MIT-LICENSE README.md package.json shakapacker.gemspec lib sig) — none of the new assets/, favicon.*, or apple-touch-icon.png files are included.
  • npm package: package.json "files" field lists only ["package", "lib/install/config/shakapacker.yml"] — new assets are excluded here as well.

No gem/package bloat from this PR.


README <picture> element ✅

The light/dark logo switcher uses the correct prefers-color-scheme media query pattern that GitHub Markdown renders natively. The <img> fallback correctly points to lockup-light.svg for environments that don't support <picture>.


SVG accessibility ✅

All SVGs have both role="img" + aria-label on the root element and a <title> child — this is the recommended pattern for maximum screen-reader compatibility.


Duplicate file: favicon.svg = assets/brand/icon-tile.svg ⚠️

These two files are byte-for-byte identical. assets/brand/README.md documents the sync expectation, which is good. Consider adding a note explaining why the root copy exists separately (Conductor icon discovery) to help future maintainers understand they should keep them in sync rather than deduplicating.


Minor issues

  • x-offset inconsistency in lockup SVGs: Both lockup-dark.svg and lockup-light.svg use x="410" for the title but x="414" for the subtitle (4px offset). Inline suggestions posted. If intentional for optical alignment, a comment in assets/brand/README.md would clarify.

Overall

Straightforward branding addition. Packaging safety is verified. Good to go once the x-offset question is resolved (or documented as intentional).

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Hero omits dark lockup variant
    • Wrapped the README hero image in a picture element with a dark-mode source pointing to lockup-dark.svg while preserving the light fallback.

Create PR

Or push these changes by commenting:

@cursor push d412285975
Preview (d412285975)
diff --git a/README.md b/README.md
--- a/README.md
+++ b/README.md
@@ -1,6 +1,10 @@
 <p align="center">
   <a href="https://shakapacker.com">
-    <img src="./assets/brand/lockup-light.png?raw=true" alt="Shakapacker: Rails asset bundling with modern build systems" width="760">
+    <picture>
+      <source media="(prefers-color-scheme: dark)" srcset="./assets/brand/lockup-dark.svg?raw=true">
+      <source media="(prefers-color-scheme: light)" srcset="./assets/brand/lockup-light.png?raw=true">
+      <img src="./assets/brand/lockup-light.png?raw=true" alt="Shakapacker: Rails asset bundling with modern build systems" width="760">
+    </picture>
   </a>
 </p>

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit f93f7a5. Configure here.

Comment thread README.md
<a href="https://shakapacker.com">
<img src="./assets/brand/lockup-light.png?raw=true" alt="Shakapacker: Rails asset bundling with modern build systems" width="760">
</a>
</p>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hero omits dark lockup variant

Low Severity

The README hero is a single light lockup PNG, while this change adds lockup-dark.svg and the PR describes a light/dark hero; on dark-themed viewers the bright light lockup is shown with no prefers-color-scheme alternative.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f93f7a5. Configure here.

@justin808
justin808 marked this pull request as ready for review May 24, 2026 01:50
@justin808
justin808 merged commit e3ac6a9 into main May 24, 2026
6 of 7 checks passed
@justin808
justin808 deleted the jg-conductor/shakapacker-icons-issue branch May 24, 2026 01:50
@claude

claude Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Code review for PR 1135 - Add Shakapacker Brand Assets. See the full review in the next comment.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f93f7a53ce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread README.md
@@ -1,3 +1,9 @@
<p align="center">
<a href="https://shakapacker.com">
<img src="./assets/brand/lockup-light.png?raw=true" alt="Shakapacker: Rails asset bundling with modern build systems" width="760">

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Point README logo to a published asset URL

The new top-of-file logo uses a relative path (./assets/brand/lockup-light.png?raw=true), but that file is not shipped in the npm package, so the README image will break in npm-rendered docs. I verified this by running npm pack --dry-run --json, which includes README.md but no assets/brand/* files; consumers viewing the package README will therefore see a broken banner unless the image is referenced via an absolute hosted URL (or the asset is included in published files).

Useful? React with 👍 / 👎.

@claude

claude Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Code Review: Add Shakapacker Brand Assets

Overview

This PR adds the Shakapacker folded-S brand identity: icon sizes in PNG, SVG mark/tile assets, light/dark lockups, and a brand lockup image near the top of README.md. Root-level discovery files (apple-touch-icon.png, favicon.png, favicon.svg) are provided for tooling like Conductor. Entirely additive — no runtime code paths touched.


What's Good

  • No distribution bloat. Confirmed: the gemspec uses an explicit allowlist, and package.json has a files field. None of the new assets will be included in the published gem or npm package.
  • Discovery copies are byte-for-byte identical to their canonical sources. diff confirms all four pairs match: favicon.svg == icon-tile.svg, apple-touch-icon.png == icon-256.png, favicon.png == icon-32.png, assets/icon.png == icon-1024.png.
  • SVGs are clean. No script tags, no xlink:href external references, no event handlers. Pure declarative graphics — no XSS risk.
  • Accessibility attributes present on all SVGs: role=img, aria-label, and title elements.
  • Total binary weight is modest (~107 KB across all PNGs), acceptable for a repo landing page.

Suggestions

1. Consider dark-mode support in the README via the picture element.

The README hardcodes the light lockup PNG. GitHub supports dark/light mode switching via picture+source. The dark SVG (lockup-dark.svg) already exists in this PR. Not blocking — the light-only approach is functional — but worth considering for GitHub users on dark themes. Example: use a picture element with a source tag pointing to lockup-dark.svg for prefers-color-scheme: dark, and the existing PNG as the img fallback.

2. Consider using SVG for the README lockup instead of PNG.

lockup-light.svg already exists and scales perfectly at any viewport width without being a 32 KB fixed-resolution raster. SVG is rendered natively by GitHub without any ?raw=true query parameter needed.

3. The ?raw=true suffix on the image path is redundant.

For relative image paths in a GitHub README.md, GitHub resolves them correctly without ?raw=true. It will not cause a problem, but it is unnecessary and could confuse future maintainers.

4. No automation to enforce discovery-copy sync.

assets/brand/README.md documents the manual sync requirement. The copies are correct right now (verified via diff), but a future contributor updating only the canonical source could silently break parity. A lightweight CI check — a shell script diffing the four pairs — would make this self-enforcing. Low priority given how infrequently brand assets change.


Minor Notes

  • lockup-dark.svg is added but has no raster PNG counterpart. assets/brand/README.md explicitly notes this is intentional. Fine.
  • Root-level favicon files are unconventional for a library repo, but the Conductor discovery rationale is documented and they do not affect gem or npm distributions.

Summary

Low risk, well-validated, zero impact on distributions. The main improvement opportunity is dark-mode README support via the picture element — the SVG assets are already in this PR. Suggestions 2–4 are minor polish items.

@greptile-apps

greptile-apps Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds the Shakapacker folded-S brand asset set: PNG icon sizes (16–1024px), SVG mark/tile/lockup assets, and root discovery copies (favicon.svg, favicon.png, apple-touch-icon.png, assets/icon.png). The light lockup is placed at the top of README.md to brand the GitHub landing page immediately.

  • Both the gemspec (git ls-files allowlist) and package.json (files allowlist covering only package/ and one config file) correctly exclude all new brand assets from published packages — no accidental size increase to gem or npm releases.
  • favicon.svg includes proper accessibility markup (role, aria-label, <title>) and the SVG path geometry is self-consistent.
  • README.md references only the light lockup; the co-shipped lockup-dark.svg is unused there, leaving dark-mode GitHub visitors with a light-background image on a dark page.

Confidence Score: 4/5

Safe to merge — purely static brand assets with no runtime code changes; published packages are unaffected.

All changes are documentation and binary/SVG assets. The gemspec and package.json both use explicit allowlists that exclude the new files from published artifacts. The only actionable finding is that the README shows the light lockup to dark-mode GitHub visitors, since the dark variant shipped in the same PR is not wired up.

README.md — the light-only lockup image could be upgraded to a picture element now that the dark SVG is available.

Important Files Changed

Filename Overview
README.md Adds centered brand lockup at the top; uses lockup-light.png only, with no dark-mode variant despite lockup-dark.svg being available in the same PR.
assets/brand/README.md New discovery-copy sync guide for brand assets; clear and correct.
favicon.svg New root favicon SVG with proper viewBox, accessibility attributes (role/aria-label/title), and correct path data for the folded-S mark.
assets/brand/lockup-dark.svg New dark lockup SVG asset; exists but not referenced in README.md for dark-mode GitHub users.
assets/brand/icon-tile.svg New canonical icon-tile SVG; source for favicon.svg and favicon.png discovery copies.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["assets/brand/ (canonical)"] --> B["icon-tile.svg"]
    A --> C["mark-transparent.svg"]
    A --> D["lockup-light.svg / .png"]
    A --> E["lockup-dark.svg"]
    A --> F["icon-16/32/64/128/256/512/1024.png"]

    B -->|copy| G["favicon.svg (root)"]
    F -->|icon-32| H["favicon.png (root)"]
    F -->|icon-256| I["apple-touch-icon.png (root)"]
    F -->|icon-1024| J["assets/icon.png"]

    D -->|used in| K["README.md img src"]
    E -.->|not referenced| K

    K --> L["GitHub README rendering"]
Loading

Reviews (1): Last reviewed commit: "Fix README logo display" | Re-trigger Greptile

Comment thread README.md
Comment on lines +1 to +5
<p align="center">
<a href="https://shakapacker.com">
<img src="./assets/brand/lockup-light.png?raw=true" alt="Shakapacker: Rails asset bundling with modern build systems" width="760">
</a>
</p>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 GitHub natively supports light/dark mode image switching via the <picture> element with prefers-color-scheme media queries. Since lockup-dark.svg is included in this same PR, dark-mode GitHub users will see the light-background lockup on a dark page. Using a <picture> element would serve the correct asset automatically.

Suggested change
<p align="center">
<a href="https://shakapacker.com">
<img src="./assets/brand/lockup-light.png?raw=true" alt="Shakapacker: Rails asset bundling with modern build systems" width="760">
</a>
</p>
<p align="center">
<a href="https://shakapacker.com">
<picture>
<source media="(prefers-color-scheme: dark)" srcset="./assets/brand/lockup-dark.svg">
<img src="./assets/brand/lockup-light.png?raw=true" alt="Shakapacker: Rails asset bundling with modern build systems" width="760">
</picture>
</a>
</p>

Comment thread README.md
@@ -1,3 +1,9 @@
<p align="center">
<a href="https://shakapacker.com">
<img src="./assets/brand/lockup-light.png?raw=true" alt="Shakapacker: Rails asset bundling with modern build systems" width="760">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ?raw=true query parameter is redundant for relative image paths in GitHub READMEs — GitHub resolves them correctly without it.

Also worth considering: since lockup-dark.svg is already in this PR, you could support GitHub's dark theme with a <picture> element:

Suggested change
<img src="./assets/brand/lockup-light.png?raw=true" alt="Shakapacker: Rails asset bundling with modern build systems" width="760">
<picture>
<source media="(prefers-color-scheme: dark)" srcset="./assets/brand/lockup-dark.svg">
<img src="./assets/brand/lockup-light.svg" width="760">
</picture>

Using the SVG variant for the light fallback would also give crisper rendering at all zoom levels compared to the 32 KB PNG raster.

@claude

claude Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Code Review

Overall: Clean, low-risk brand asset addition. The SVG files are well-formed and the sync documentation in assets/brand/README.md is clear. A few minor observations:

Minor Issues

?raw=true on relative paths is unnecessary

In README.md line 3, ./assets/brand/lockup-light.png?raw=true — the ?raw=true query parameter only has effect when used with a full GitHub blob URL (e.g. https://github.com/.../blob/main/file.png?raw=true). On a relative path, GitHub's markdown renderer already serves the binary directly. This is harmless but could confuse future editors.

Enhancement Opportunity

README could support dark mode with <picture>

Since both lockup-light and lockup-dark assets exist, GitHub's markdown renderer supports theme-adaptive images via the <picture> element. This would show the dark lockup to users with dark mode enabled:

<picture>
  <source media="(prefers-color-scheme: dark)" srcset="./assets/brand/lockup-dark.svg">
  <img src="./assets/brand/lockup-light.png" alt="Shakapacker: Rails asset bundling with modern build systems" width="760">
</picture>

The lockup-dark.svg SVG can be used directly here — no raster dark variant needed.

What's Good

  • All SVG files include proper accessibility attributes (role="img", aria-label, <title> element).
  • favicon.svg and assets/brand/icon-tile.svg are byte-for-byte identical as documented — the sync invariant holds.
  • assets/brand/README.md clearly documents which discovery copies map to which canonical source, making future updates straightforward.
  • Trailing newlines are present on all text files.

justin808 added a commit that referenced this pull request May 24, 2026
…coverage-v1

* origin/main:
  Teach /update-changelog to drop RC-only regression fixes (#1125)
  Restructure supplemental package dependencies (#1131) (#1133)
  [codex] Add Shakapacker brand assets (#1135)
  [codex] Fix shakapacker config helper binstubs (#1132)
  Surface v10.1 supplemental packages from README and key docs (#1134)
  CHANGELOG: merge PR #1120 into PR #1110 entry for v10.1.0-rc.1 (#1126)

# Conflicts:
#	lib/install/bin/diff-bundler-config
#	lib/install/bin/shakapacker-config
#	package/configExporter/cli.ts
#	spec/dummy/bin/shakapacker-config
#	spec/shakapacker/binstub_sync_spec.rb
#	spec/shakapacker/helper_binstubs_spec.rb
justin808 added a commit that referenced this pull request Jun 9, 2026
…nalyze-webpack-output

* origin/main: (164 commits)
  Make Rspack the default bundler for new installs (#1150)
  Add "Why Shakapacker with Rspack" comparison guide (#1155)
  Support sass-loader v17 (#1141)
  [codex] Fix Claude workflow permissions (#1153)
  Fix README CI badges and add prominent docs link (#1152)
  Trim README into a docs-site index; relocate content to docs/ and fix deep links (#1148)
  Tighten helper binstub parity and PATH lookup (#1128)
  Optimize CI: minimal version matrix on PRs, full matrix on main (#1151)
  Fix instance-scoped compiler strategies (#1147)
  Fix Rspack devServer in static watch mode (#1142)
  Add homepage links for supplemental npm packages (#1145)
  Release 10.1.0
  Update CHANGELOG.md for v10.1.0 (#1139)
  Expand prerelease-to-stable changelog coalesce guidance in /update-changelog (#1138)
  Release 10.1.0-rc.2
  [codex] Update changelog for v10.1.0-rc.2 (#1136)
  Teach /update-changelog to drop RC-only regression fixes (#1125)
  Restructure supplemental package dependencies (#1131) (#1133)
  [codex] Add Shakapacker brand assets (#1135)
  [codex] Fix shakapacker config helper binstubs (#1132)
  ...
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.

1 participant