Skip to content

Comments

feat: add astro content#1

Merged
Pertempto merged 4 commits intomainfrom
ae-add-astro-content
Nov 4, 2025
Merged

feat: add astro content#1
Pertempto merged 4 commits intomainfrom
ae-add-astro-content

Conversation

@Pertempto
Copy link
Contributor

@Pertempto Pertempto commented Nov 4, 2025

Added example files for Astro.

@Pertempto Pertempto requested a review from bambam955 November 4, 2025 13:55
@Pertempto Pertempto self-assigned this Nov 4, 2025
@github-actions
Copy link

github-actions bot commented Nov 4, 2025

@Pertempto The added package.json is missing common metadata which can cause tooling or publishing issues. Please add at minimum:

  • name, version, and license (e.g. "name": "docs-template", "version": "0.0.0", "license": "MIT").
  • Consider private: true if this repo should not be published.
  • Consider adding engines.node to lock Node version.

Also: sharp is a native dependency that often requires build tools on CI/Developer machines. Either document this in the README (prereqs) or consider making it an optionalDependency or provide an installation note.

@github-actions
Copy link

github-actions bot commented Nov 4, 2025

@Pertempto Please add a lockfile to the PR (e.g. package-lock.json, pnpm-lock.yaml, or yarn.lock). The PR adds package.json but no lockfile — this makes installs non-deterministic and can cause CI/build differences. Add the appropriate lockfile and include it in the commit.

@github-actions
Copy link

github-actions bot commented Nov 4, 2025

Changes Requested

Please make the following changes before I can approve:

  • Confirm and commit a single authoritative lockfile (or state the package manager you expect CI to use); ensure the lockfile was generated with the intended npm/pnpm/yarn version.
  • Add minimal package.json metadata: a repository field and engines.node (e.g. "engines": { "node": ">=18" }). Keeping private: true is fine for a template.
  • Address sharp: either move it to optionalDependencies or document system/CI prerequisites (libvips / build tools) in the README.
  • Make /.vscode/launch.json cross-platform (use npm run dev), or remove workspace-specific launch configs from the repo.
  • Update the README to reference public/ (used by this project) instead of static/ to avoid confusion.

Summary of Changes

  • Adds Astro + Starlight scaffolding: astro.config.mjs, tsconfig.json, src/content with sample docs and src/assets.
  • Adds package.json and a package-lock.json (lockfile present in PR tree).
  • Adds many public assets (favicons, webmanifest, logos) and VS Code workspace configs under /.vscode.
  • Adds src/content.config.ts and example MD/MDX content.

Overall Feedback

  • Most important issues: the repo currently has potential install/CI fragility (lockfile expectations and the native sharp dependency) and a couple of repo-configuration items that can break contributor UX (missing repository/engines fields and a platform-specific VS Code launch task). Please address those first so builds and local dev are deterministic.

  • Nice work setting up a usable Astro + Starlight template. The example pages, content config, and assets make it straightforward for teams to fork and start writing docs — clean and practical setup! 🎉

If you want, I can re-check after you push the changes. @Pertempto

@mrs-electronics-inc mrs-electronics-inc deleted a comment from github-actions bot Nov 4, 2025
@Pertempto
Copy link
Contributor Author

The added package.json is missing common metadata which can cause tooling or publishing issues.

This is a template repository and we don't want to publish our docs repositories on npm so we don't even need those fields. I will add private: true

@Pertempto
Copy link
Contributor Author

Pertempto commented Nov 4, 2025

@Pertempto Please add a lockfile to the PR (e.g. package-lock.json, pnpm-lock.yaml, or yarn.lock). The PR adds package.json but no lockfile — this makes installs non-deterministic and can cause CI/build differences. Add the appropriate lockfile and include it in the commit.

Wrong. there is package-lock.json

@github-actions
Copy link

github-actions bot commented Nov 4, 2025

package.json: Thanks for adding private: true — that's appropriate for a template. A couple of actionable items:

  • Add a repository field so tooling and editors can link back to the repo (e.g. "repository": { "type": "git", "url": "git+https://github.com/mrs-electronics-inc/docs-template.git" }).
  • Add an engines.node field (e.g. "engines": { "node": ">=18" }) to pin Node expectations for CI and contributors.
  • Consider adding minimal name and version values to avoid tooling that expects them (optional if you keep private: true).
  • Confirm the committed package-lock.json was generated by the npm version you expect CI to use; if you plan to use pnpm/yarn instead, commit the matching lockfile instead.

@github-actions
Copy link

github-actions bot commented Nov 4, 2025

.vscode/launch.json: The current command points to a relative path: ./node_modules/.bin/astro dev. This is platform-specific and brittle in some environments. Actionable suggestions:

  • Replace command with a cross-platform npm run dev and set runtimeExecutable if needed, or use the console + args fields to call the dev script.
  • Alternatively, remove workspace-specific launch configs from the repo if they aren't meant for all contributors. If kept, document usage in README.

Copy link

@bambam955 bambam955 left a comment

Choose a reason for hiding this comment

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

Looks great! I like where you're going with this 👍

@Pertempto Pertempto merged commit 046f357 into main Nov 4, 2025
1 check passed
@Pertempto Pertempto deleted the ae-add-astro-content branch November 4, 2025 14:06
@bambam955 bambam955 mentioned this pull request Nov 4, 2025
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