Skip to content

Comments

feat: basic configuration#3

Merged
Pertempto merged 5 commits intomainfrom
ae-basic-setup
Nov 4, 2025
Merged

feat: basic configuration#3
Pertempto merged 5 commits intomainfrom
ae-basic-setup

Conversation

@Pertempto
Copy link
Contributor

Basic configuration of the Astro site.

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

github-actions bot commented Nov 4, 2025

README.md: You changed the build output in the commands table to dist/. Please confirm that npm run build actually outputs to dist/ and that your CI / hosting (GitHub Pages, Actions, or other) expects dist/. If the build produces public/ (or your hosting expects public/), either update the build script or revert this README change so they match.

@github-actions
Copy link

github-actions bot commented Nov 4, 2025

astro.config.mjs: You changed site to https://mconn.mrs-electronics.dev and updated the Starlight title and social href. Please confirm this is the canonical production URL and that your deployment (CI/GitHub Pages/DNS) is configured for it. Also verify the GitHub link should point to the repo (/mrs-electronics-inc/mconn-docs) — restore any other social links intentionally removed.

@github-actions
Copy link

github-actions bot commented Nov 4, 2025

Changes Requested

Please make the following changes before merging:

  • Ensure build output is consistent: make npm run build actually output to dist/ (or revert README.md to the folder your build produces). Also update any CI/GitHub Actions that publish the site to expect the same folder.
  • Fix package name mismatch: add the name field to package.json (e.g. "mconn-docs") and regenerate package-lock.json by running npm install, then commit the updated lockfile. Do not hand-edit the lockfile.
  • Confirm canonical site and repo links in astro.config.mjs: if site: "https://mconn.mrs-electronics.dev" and the GitHub href are intentional, add a one-line inline comment in astro.config.mjs or a short note in the PR description confirming DNS/CI are configured for that domain.

Summary of Changes

  • README.md: renamed project, updated host URL, changed static assets dir to public/, and changed documented build output to dist/.
  • astro.config.mjs: set site, Starlight title, and GitHub social href to MConn values.
  • package-lock.json: name changed to mconn-docs (lockfile needs to be regenerated, not hand-edited).
  • package.json: added name field (mconn-docs).
  • public/site.webmanifest: name and short_name updated.
  • src/content/docs/index.mdx: title changed to MConn Documentation.

Overall Feedback

  • Immediate concerns: The build output mismatch and the package name vs lockfile mismatch can cause deployment failures and confusing CI behavior. Please fix those first so the repo builds and deploys cleanly.

  • Positive: This is a focused, small PR that correctly replaces template placeholders with project-specific values — good progress toward a live docs site. Once the three items above are addressed, this should be ready to merge. Nice work! 🎉

@Pertempto — ping me after you push the fixes and I’ll re-check quickly.

@github-actions
Copy link

github-actions bot commented Nov 4, 2025

README.md: Please confirm whether npm run build actually outputs to dist/ (you changed the README from ./public/ to dist/). If it does not, either:

  • Update package.json build script to output to dist/ (and update any CI that expects a different folder), or
  • Revert the README line to the folder your build produces (e.g. ./public/).

If you want, paste your package.json scripts block and I can suggest the exact change.

@Pertempto
Copy link
Contributor Author

I've confirmed it locally.

Copy link
Contributor

@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 good so far...make sure any necessary changes (i.e. the public vs dist stuff) are made in mrs-electronics-inc/docs-template#3 too.

@Pertempto Pertempto merged commit b0f0386 into main Nov 4, 2025
1 check passed
@Pertempto Pertempto deleted the ae-basic-setup branch November 4, 2025 14:47
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