kwila/market
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

chore: tooling #16

Merged
Pertempto merged 4 commits from claude/take-a-look-01AYMg36C7FjSM6RT2P84Mr6 into main 2025-11-14 10:07:23 -05:00
Conversation 7 Commits 4 Files changed 18 +6994 -267
Pertempto commented 2025-11-13 19:47:11 -05:00 (Migrated from github.com)
Copy link
  • Setup ESLint with TypeScript, React, and Astro support
    • Configured separate rules for TS, JSX, and Astro files
    • Added accessibility checks with jsx-a11y
  • Setup Prettier with Astro plugin for consistent formatting
  • Added GitHub Actions CI workflow with:
    • Linting job
    • Type checking job
    • Format verification job
  • Configured PWA with @vite-pwa/astro
    • Added web app manifest for installability
    • Configured service worker with Workbox
    • Added runtime caching for fonts
  • Added npm scripts for lint, format, and type-check
  • Updated MVP spec to mark infrastructure tasks as complete

Note: PWA icon files (icon-192.png, icon-512.png) need to be generated

- Setup ESLint with TypeScript, React, and Astro support - Configured separate rules for TS, JSX, and Astro files - Added accessibility checks with jsx-a11y - Setup Prettier with Astro plugin for consistent formatting - Added GitHub Actions CI workflow with: - Linting job - Type checking job - Format verification job - Configured PWA with @vite-pwa/astro - Added web app manifest for installability - Configured service worker with Workbox - Added runtime caching for fonts - Added npm scripts for lint, format, and type-check - Updated MVP spec to mark infrastructure tasks as complete Note: PWA icon files (icon-192.png, icon-512.png) need to be generated
github-actions[bot] commented 2025-11-13 19:49:53 -05:00 (Migrated from github.com)
Copy link

@Pertempto CI will fail: the workflow uses npm ci but there is no package-lock.json in the repo. Either commit the lockfile or change the workflow to npm install (or npm ci only on jobs that have a lockfile).

@Pertempto CI will fail: the workflow uses `npm ci` but there is no `package-lock.json` in the repo. Either commit the lockfile or change the workflow to `npm install` (or `npm ci` only on jobs that have a lockfile).
github-actions[bot] commented 2025-11-13 19:50:48 -05:00 (Migrated from github.com)
Copy link

@Pertempto In astro.config.mjs you're importing tailwindcss from @tailwindcss/vite, but I don't see that package in package.json. This will break the build. Please either:

  • Switch to the official Astro integration: import tailwind from '@astrojs/tailwind' and add tailwind() to integrations, or
  • Add the correct Vite Tailwind plugin as a devDependency and ensure the import name matches the package you install.

Either way, ensure package.json is updated so CI can install the dependency.

@Pertempto In `astro.config.mjs` you're importing `tailwindcss` from `@tailwindcss/vite`, but I don't see that package in `package.json`. This will break the build. Please either: - Switch to the official Astro integration: `import tailwind from '@astrojs/tailwind'` and add `tailwind()` to `integrations`, or - Add the correct Vite Tailwind plugin as a devDependency and ensure the import name matches the package you install. Either way, ensure `package.json` is updated so CI can install the dependency.
github-actions[bot] commented 2025-11-13 19:51:12 -05:00 (Migrated from github.com)
Copy link

@Pertempto eslint.config.js exports an array but also spreads astroPlugin.configs.recommended directly into the top-level array. The Astro plugin provides an object config; mixing it with the array may produce an invalid config shape for ESLint. Consider building a single config object with overrides for file types, or ensure the final exported value matches ESLint's expected format (object with overrides or extends). Also verify all imported plugin objects (like react.configs.recommended) are valid to spread — some exports may not be plain rule objects.

@Pertempto `eslint.config.js` exports an array but also spreads `astroPlugin.configs.recommended` directly into the top-level array. The Astro plugin provides an object config; mixing it with the array may produce an invalid config shape for ESLint. Consider building a single config object with `overrides` for file types, or ensure the final exported value matches ESLint's expected format (object with `overrides` or `extends`). Also verify all imported plugin objects (like `react.configs.recommended`) are valid to spread — some exports may not be plain rule objects.
github-actions[bot] commented 2025-11-13 19:51:37 -05:00 (Migrated from github.com)
Copy link

Changes Requested

Please address these blocking issues before merging:

  • Fix CI install step: update each Install dependencies step in .github/workflows/ci.yml to run npm ci only when a lockfile exists or fall back to npm install.
  • Resolve Tailwind integration in astro.config.mjs: either switch to @astrojs/tailwind (add tailwind() to integrations) or install the correct Vite Tailwind plugin matching your import and update package.json.
  • Correct eslint.config.js: do not spread plugin configs.* into the top-level array. Export a single flat-config object using overrides for file types and reference plugin rules explicitly.
  • Add the PWA icons referenced in astro.config.mjs (public/icon-192.png, public/icon-512.png) or update the manifest to point to existing assets.

Summary of Changes

  • Added CI workflow at .github/workflows/ci.yml (lint, type-check, format-check).
  • Added Prettier config and .prettierignore.
  • Introduced ESLint flat config eslint.config.js (needs shape fixes).
  • Configured PWA integration in astro.config.mjs and added public/ICONS-TODO.md.
  • Updated package.json with new scripts and devDependencies (some mismatches exist).
  • Updated many docs/spec files and the landing page (src/pages/index.astro).

Overall Feedback

  • Negative: There are a few immediate, blocking issues that will cause CI and local builds to fail: the workflow unconditionally uses npm ci without a lockfile, astro.config.mjs imports a Tailwind Vite plugin that is not present in package.json, and eslint.config.js spreads plugin config objects which likely produce an invalid ESLint flat config. These should be fixed before merging.

  • Positive: The PR adds valuable developer tooling (CI, ESLint, Prettier), PWA scaffolding, and a clean landing page—this will improve developer experience once the above issues are resolved. Nice work adding these infra pieces!

If you want, I can open a small follow-up with example diffs for the CI install snippet, a safe astro.config.mjs Tailwind integration using @astrojs/tailwind, and a corrected eslint.config.js skeleton.

@Pertempto

# Changes Requested Please address these blocking issues before merging: - Fix CI install step: update each `Install dependencies` step in `.github/workflows/ci.yml` to run `npm ci` only when a lockfile exists or fall back to `npm install`. - Resolve Tailwind integration in `astro.config.mjs`: either switch to `@astrojs/tailwind` (add `tailwind()` to `integrations`) or install the correct Vite Tailwind plugin matching your import and update `package.json`. - Correct `eslint.config.js`: do not spread plugin `configs.*` into the top-level array. Export a single flat-config object using `overrides` for file types and reference plugin rules explicitly. - Add the PWA icons referenced in `astro.config.mjs` (`public/icon-192.png`, `public/icon-512.png`) or update the manifest to point to existing assets. ## Summary of Changes - Added CI workflow at `.github/workflows/ci.yml` (lint, type-check, format-check). - Added Prettier config and `.prettierignore`. - Introduced ESLint flat config `eslint.config.js` (needs shape fixes). - Configured PWA integration in `astro.config.mjs` and added `public/ICONS-TODO.md`. - Updated `package.json` with new scripts and devDependencies (some mismatches exist). - Updated many docs/spec files and the landing page (`src/pages/index.astro`). ## Overall Feedback - Negative: There are a few immediate, blocking issues that will cause CI and local builds to fail: the workflow unconditionally uses `npm ci` without a lockfile, `astro.config.mjs` imports a Tailwind Vite plugin that is not present in `package.json`, and `eslint.config.js` spreads plugin config objects which likely produce an invalid ESLint flat config. These should be fixed before merging. - Positive: The PR adds valuable developer tooling (CI, ESLint, Prettier), PWA scaffolding, and a clean landing page—this will improve developer experience once the above issues are resolved. Nice work adding these infra pieces! If you want, I can open a small follow-up with example diffs for the CI install snippet, a safe `astro.config.mjs` Tailwind integration using `@astrojs/tailwind`, and a corrected `eslint.config.js` skeleton. @Pertempto
Pertempto commented 2025-11-13 19:55:12 -05:00 (Migrated from github.com)
Copy link

@claude please fix the problems raised in the code review

@claude please fix the problems raised in the code review
👀 1
cloudflare-workers-and-pages[bot] commented 2025-11-13 20:02:35 -05:00 (Migrated from github.com)
Copy link

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs		 market 4b28c690 Commit Preview URL

Branch Preview URL		 Nov 14 2025, 02:54 PM
## Deploying with &nbsp;<a href="https://workers.dev"><img alt="Cloudflare Workers" src="https://workers.cloudflare.com/logo.svg" width="16"></a> &nbsp;Cloudflare Workers The latest updates on your project. Learn more about [integrating Git with Workers](https://developers.cloudflare.com/workers/ci-cd/builds/git-integration/). | Status | Name | Latest Commit | Preview URL | Updated (UTC) | | -|-|-|-|-| | ✅ Deployment successful! <br>[View logs](https://dash.cloudflare.com/?to=/ef1b3f6f45e7d16874284a2637bec85b/workers/services/view/market/production/builds/a2a057b1-04f8-4035-9ff9-f533ed78d419) | market | 4b28c690 | <a href='https://f1aa55cf-market.kwila-cloud.workers.dev'>Commit Preview URL</a><br><br><a href='https://claude-take-a-look-01aymg36c7fjsm6rt2p84mr6-market.kwila-cloud.workers.dev'>Branch Preview URL</a> | Nov 14 2025, 02:54 PM |
github-actions[bot] commented 2025-11-14 09:51:07 -05:00 (Migrated from github.com)
Copy link

@Pertempto — CI install step will fail when there's no lockfile. Please change the install step to run npm ci only when a lockfile is present or switch to npm install. Quick, non-breaking fix you can use in each job:

run: |
if [ -f package-lock.json ]; then
npm ci
else
npm install
fi

This avoids forcing a lockfile commit and keeps CI deterministic when a lockfile exists.

@Pertempto — CI install step will fail when there's no lockfile. Please change the install step to run `npm ci` only when a lockfile is present or switch to `npm install`. Quick, non-breaking fix you can use in each job: run: | if [ -f package-lock.json ]; then npm ci else npm install fi This avoids forcing a lockfile commit and keeps CI deterministic when a lockfile exists.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
kwila/market!16
No description provided.