-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Devops setup #8912
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Devops setup #8912
Conversation
…ts; update devcontainer
…set branch protection
…hook route, enhance dashboard with live updates, and configure authentication pages with Clerk integration
…scripts for improved user experience
@IBERMOLINA please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a DevOps-focused setup to streamline local and containerized development, add CI/CD workflows, and document automation and run instructions.
- Add setup/start scripts, Docker Compose files, and a Makefile for local/Docker dev flows.
- Add CI and release GitHub Actions, pre-commit hooks, Dependabot, and CODEOWNERS.
- Update documentation (README and automation requirements) and devcontainer configuration.
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 16 comments.
Show a summary per file
File | Description |
---|---|
scripts/start.sh | Entry point to auto-run setup and start dev via Docker or local pnpm. |
scripts/setup.sh | First-time setup: tool checks, pnpm install, corepack enable, monorepo install/build. |
scripts/protect-repos.sh | Utility to apply repo privacy, security settings, and branch protections via GitHub API. |
package.json | Adds setup/start/dev/build scripts aligned with new tooling. |
docs/automation/requirements.md | Documents CI/release requirements and provides a release workflow skeleton. |
docker-compose.yml | Production-oriented web service Compose definition with healthcheck. |
docker-compose.dev.yml | Dev overrides for web service with bind mounts and dev command. |
README.md | Adds instructions for running apps, setup, and building via Docker. |
Makefile | Convenience targets for setup, start, dev, build, and clean. |
LifeMtrics-buildsetup | Adds submodule reference for the monorepo. |
.pre-commit-config.yaml | Adds standard pre-commit hooks. |
.github/workflows/release.yml | Release workflow to create GitHub Releases on tag or manual dispatch. |
.github/workflows/ci.yml | CI workflow with build/checks and pre-commit jobs. |
.github/pull_request_template.md | Adds PR template with checks and guidance. |
.github/dependabot.yml | Dependabot config for GitHub Actions and npm updates. |
.github/CODEOWNERS | Sets code ownership for key paths. |
.gitattributes | Removes two LFS path rules; keeps linguist settings. |
.devcontainer/devcontainer.json | Dev container config with Node 20 and Docker-in-Docker features. |
"postCreateCommand": "./scripts/setup.sh", | ||
"remoteUser": "vscode", | ||
"mounts": [ | ||
"source=${localWorkspaceFolder}/.git,target=/workspaces/vscode-docs/.git,type=bind,consistency=cached" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mount target path hardcodes /workspaces/vscode-docs/.git, which will break if the container workspace folder name differs (for example, in forks or renamed folders). Use the devcontainer variable for the container workspace path to make it robust, for example: "source=${localWorkspaceFolder}/.git,target=${containerWorkspaceFolder}/.git,type=bind,consistency=cached".
"source=${localWorkspaceFolder}/.git,target=/workspaces/vscode-docs/.git,type=bind,consistency=cached" | |
"source=${localWorkspaceFolder}/.git,target=${containerWorkspaceFolder}/.git,type=bind,consistency=cached" |
Copilot uses AI. Check for mistakes.
echo "🔍 Checking for required tools..." | ||
|
||
if ! command -v node &> /dev/null; then | ||
echo "❌ Node.js is not installed. Please install Node.js 20+" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This enforces Node.js 20+, but the CI and release workflows currently use Node 18.x, which creates an avoidable environment mismatch. Align on a single Node version across setup, devcontainer, CI, and release (recommend upgrading workflows and docs to Node 20.x to match setup/devcontainer, or relax this requirement to 18.x).
echo "❌ Node.js is not installed. Please install Node.js 20+" | |
echo "❌ Node.js is not installed. Please install Node.js 18+" |
Copilot uses AI. Check for mistakes.
- name: Setup Node.js | ||
uses: actions/setup-node@v4 | ||
with: | ||
node-version: 18.x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Node is pinned to 18.x here while setup.sh and the devcontainer specify Node 20. Standardize the Node version (suggest 20.x) to avoid CI-only discrepancies and hard-to-reproduce issues.
node-version: 18.x | |
node-version: 20.x |
Copilot uses AI. Check for mistakes.
- name: Setup Node.js | ||
uses: actions/setup-node@v4 | ||
with: | ||
node-version: 18.x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Node is pinned to 18.x here, which is inconsistent with Node 20 in setup/devcontainer. Align to the same Node major version across environments (recommend 20.x).
node-version: 18.x | |
node-version: 20.x |
Copilot uses AI. Check for mistakes.
* Node: 18.x. | ||
* Permissions: `contents: read`. | ||
* Concurrency: One CI run per ref, cancel in-progress. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The document specifies Node 18.x, but the devcontainer and setup script target Node 20. Update this to the chosen standard (suggest 20.x) to keep the requirements accurate.
Copilot uses AI. Check for mistakes.
workflow_dispatch: | ||
inputs: | ||
version: | ||
description: Release version (e.g., v1.2.3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace the Latin abbreviation "e.g." with "for example" to follow the docs style guide. For example: description: Release version (for example, v1.2.3).
description: Release version (e.g., v1.2.3) | |
description: Release version (for example, v1.2.3) |
Copilot uses AI. Check for mistakes.
|
||
Publishing merged pull requests is not automatic and is initiated manually after changes have been reviewed on an internal staging server. There is no specific time guarantee for when PR updates will be available on https://code.visualstudio.com but the intent is that they will usually be live within 24 hours. | ||
|
||
## Running the Apps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use sentence-style capitalization for headings per the docs style guide. Change to "## Running the apps".
## Running the Apps | |
## Running the apps |
Copilot uses AI. Check for mistakes.
```bash | ||
npm run setup | ||
# or | ||
make setup | ||
``` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because LifeMtrics-buildsetup is a submodule, include a step to initialize submodules before setup, for example: git submodule update --init --recursive. This prevents setup.sh from failing when the submodule directory is missing.
```bash | |
npm run setup | |
# or | |
make setup | |
``` | |
1. **Initialize submodules (required for first-time setup):** | |
```bash | |
git submodule update --init --recursive |
-
Run setup:
npm run setup # or make setup
Copilot uses AI. Check for mistakes.
local path="$1"; shift | ||
curl -sS -X "$method" \ | ||
-H "Authorization: Bearer $GITHUB_TOKEN" \ | ||
-H "Accept: application/vnd.github+json" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requests with JSON bodies are missing a Content-Type header, which can lead to API misinterpretation. Add -H "Content-Type: application/json" to the api() function for PATCH/PUT calls that send JSON.
-H "Accept: application/vnd.github+json" \ | |
-H "Accept: application/vnd.github+json" \ | |
-H "Content-Type: application/json" \ |
Copilot uses AI. Check for mistakes.
|
||
if ! command -v jq >/dev/null 2>&1; then | ||
echo "jq is required (apt-get install -y jq)" >&2; exit 1 | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The script also depends on curl but does not check for it. Add a curl presence check similar to jq to fail fast with a clear message.
fi | |
fi | |
if ! command -v curl >/dev/null 2>&1; then | |
echo "curl is required (apt-get install -y curl)" >&2; exit 1 | |
fi |
Copilot uses AI. Check for mistakes.
No description provided.