[WIP] Add issue and PR templates and CI workflows#285
[WIP] Add issue and PR templates and CI workflows#285danielmarv merged 3 commits into239-newsletter-page-rendering-and-functionalityfrom
Conversation
7463cd7 to
bc64d63
Compare
9e0b94b
into
239-newsletter-page-rendering-and-functionality
There was a problem hiding this comment.
Pull request overview
This pull request introduces GitHub templates and CI workflows to improve automation and standardize issue/PR management. The PR is marked as WIP (Work in Progress) with a checklist identifying several issues that need to be fixed in the workflow files.
Changes:
- Added CI workflow for automated testing and building
- Added PR automation workflow for auto-labeling and assignment
- Added issue automation workflow for auto-labeling bug reports and feature requests
- Added PR and issue templates to standardize contributions
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/ci.yml | New CI workflow for linting, type-checking, testing, and building |
| .github/workflows/pr-automation.yml | New automation for auto-labeling PRs based on changed files |
| .github/workflows/issue-automation.yml | New automation for auto-labeling and assigning issues |
| .github/pull_request_template.md | New template to standardize pull request descriptions |
| .github/ISSUE_TEMPLATE/bug_report.yml | New structured template for bug reports |
| .github/ISSUE_TEMPLATE/feature_request.yml | New structured template for feature requests |
| .github/ISSUE_TEMPLATE/config.yml | Configuration to disable blank issues and provide discussion link |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| branches: [main, develop] | ||
| pull_request: | ||
| branches: [main, develop] |
There was a problem hiding this comment.
The branches should include 'next-js-migration' to match the existing e2e-tests.yml workflow. The PR description checklist mentions updating branches to 'next-js-migration', and the existing workflow triggers on [main, next-js-migration].
| branches: [main, develop] | |
| pull_request: | |
| branches: [main, develop] | |
| branches: [main, develop, next-js-migration] | |
| pull_request: | |
| branches: [main, develop, next-js-migration] |
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Auto-label bug reports | ||
| if: contains(github.event.issue.title, 'bug') || contains(github.event.issue.body, 'bug') |
There was a problem hiding this comment.
The case-sensitive check 'contains(github.event.issue.title, 'bug')' will not match 'Bug', 'BUG', or other case variations. According to the PR description, toLower() should be used for case-insensitive matching. Use 'contains(toLower(github.event.issue.title), 'bug')' instead.
| if: contains(github.event.issue.title, 'bug') || contains(github.event.issue.body, 'bug') | |
| if: contains(toLower(github.event.issue.title), 'bug') || contains(toLower(github.event.issue.body), 'bug') |
| }) | ||
|
|
||
| - name: Auto-label feature requests | ||
| if: contains(github.event.issue.title, 'feature') || contains(github.event.issue.title, 'enhancement') |
There was a problem hiding this comment.
The PR description mentions adding a body check for feature detection, but the feature request detection only checks the title. Consider adding 'contains(toLower(github.event.issue.body), 'feature') || contains(toLower(github.event.issue.body), 'enhancement')' to also detect feature requests from the issue body content.
| - name: Setup pnpm | ||
| uses: pnpm/action-setup@v2 | ||
| with: | ||
| version: 8 |
There was a problem hiding this comment.
According to the PR description, the pnpm version should be updated to 10. However, the existing e2e-tests.yml workflow uses version 8. Consider whether this should be consistently updated across all workflows, or if version 8 should be retained to match the existing configuration.
| if (file.match(/test|spec/)) labels.add('tests'); | ||
| if (file.match(/package\.json|tsconfig|eslint|tailwind/)) labels.add('configuration'); | ||
| if (file.match(/README|\.md$/)) labels.add('documentation'); | ||
| if (file.match(/next\.config|public/)) labels.add('build'); |
There was a problem hiding this comment.
The build pattern 'next.config|public' is overly broad and will match any file containing these strings in their path, such as 'my_next.config_backup.txt'. According to the PR description, build regex patterns need improvement. Use more specific patterns like 'next.config.(js|ts|mjs)$|^public/' to match Next.js config files and the public directory.
| if (file.match(/next\.config|public/)) labels.add('build'); | |
| if (file.match(/(next\.config\.(js|ts|mjs)$|^public\/)/)) labels.add('build'); |
|
|
||
| files.forEach(fileObj => { | ||
| const file = fileObj.filename || ''; | ||
| if (file.match(/\.(ts|tsx|jsx|js)$/)) labels.add('typescript'); |
There was a problem hiding this comment.
The labeling logic combines TypeScript and JavaScript files under a single 'typescript' label. According to the PR description, TS/JS labels should be separated. Consider creating distinct labels for TypeScript files (.ts, .tsx) and JavaScript files (.js, .jsx).
| if (file.match(/\.(ts|tsx|jsx|js)$/)) labels.add('typescript'); | ||
| if (file.match(/\.(css|scss|postcss)$/)) labels.add('styling'); | ||
| if (file.match(/test|spec/)) labels.add('tests'); | ||
| if (file.match(/package\.json|tsconfig|eslint|tailwind/)) labels.add('configuration'); |
There was a problem hiding this comment.
The configuration pattern 'package.json|tsconfig|eslint|tailwind' is overly broad and will match any file containing these strings in their path. According to the PR description, config regex patterns need improvement. Use more specific patterns like '^package.json$|tsconfig.json$|eslint.config.(js|mjs|cjs)$|tailwind.config.(js|ts)$' to match exact configuration files.
| if (file.match(/package\.json|tsconfig|eslint|tailwind/)) labels.add('configuration'); | |
| if (file.match(/^(package\.json|tsconfig\.json|eslint\.config\.(js|mjs|cjs)|tailwind\.config\.(js|ts))$/)) labels.add('configuration'); |
| }) | ||
|
|
||
| - name: Auto-label feature requests | ||
| if: contains(github.event.issue.title, 'feature') || contains(github.event.issue.title, 'enhancement') |
There was a problem hiding this comment.
The feature request detection should use case-insensitive matching with toLower() as mentioned in the PR description. Use 'contains(toLower(github.event.issue.title), 'feature') || contains(toLower(github.event.issue.title), 'enhancement')' instead.
| if: contains(github.event.issue.title, 'feature') || contains(github.event.issue.title, 'enhancement') | |
| if: contains(toLower(github.event.issue.title), 'feature') || contains(toLower(github.event.issue.title), 'enhancement') |
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Auto-label bug reports | ||
| if: contains(github.event.issue.title, 'bug') || contains(github.event.issue.body, 'bug') |
There was a problem hiding this comment.
The condition 'contains(github.event.issue.body, 'bug')' may fail if the issue body is null or empty. GitHub issues can have an empty body. Add a null check before accessing the body: 'contains(github.event.issue.title, 'bug') || (github.event.issue.body && contains(github.event.issue.body, 'bug'))'
| if: contains(github.event.issue.title, 'bug') || contains(github.event.issue.body, 'bug') | |
| if: contains(github.event.issue.title, 'bug') || (github.event.issue.body && contains(github.event.issue.body, 'bug')) |
| cache: 'pnpm' | ||
|
|
||
| - name: Install dependencies | ||
| run: pnpm install --frozen-lockfile |
There was a problem hiding this comment.
The existing e2e-tests.yml workflow uses 'pnpm install --no-frozen-lockfile', while this CI workflow uses '--frozen-lockfile'. For CI consistency and to prevent builds from failing due to lockfile mismatches, '--frozen-lockfile' is generally the better choice as it ensures the lockfile matches package.json exactly. However, consider whether this should be aligned with the existing workflow convention, or if the existing workflow should be updated as well.
| run: pnpm install --frozen-lockfile | |
| run: pnpm install --no-frozen-lockfile |
ci.yml: Update pnpm action to v4, pnpm version to 10, Node.js to 22.x, branches tonext-js-migration, and use--if-presentflagpr-automation.yml: Separate TS/JS labels, fix CSS/postcss pattern, improve test/config/docs/build regex patternsissue-automation.yml: UsetoLower()for case-insensitive matching, add body check for feature detection💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.