Skip to content

Add PR template and eslint step to github action#109

Open
seanlim wants to merge 3 commits into
masterfrom
feat/add-pr-template-and-ci
Open

Add PR template and eslint step to github action#109
seanlim wants to merge 3 commits into
masterfrom
feat/add-pr-template-and-ci

Conversation

@seanlim

@seanlim seanlim commented Jun 25, 2026

Copy link
Copy Markdown
Owner

Description

  • added lint step to PR github action
  • added PR template
  • update eslint.config.mjs to ignore generated coverage artifacts
  • fix lint errors

Testing

Ensure GitHub actions run and actually lint and run tests.

Reviewer Notes

  • PR template at .github/pull_request_template.md
  • Main changes to add linting to PR github action at .github/workflows/test.yml
  • The rest of the changes are fixes for linting errors

@seanlim seanlim changed the title Feat/add pr template and eslint step to github action Add PR template and eslint step to github action Jun 25, 2026

@ngkhengyang ngkhengyang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, but I think the way the workflow is run can be changed slightly

@@ -1,10 +1,10 @@
name: Tests
name: Lint and Test

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
name: Lint and Test
name: ci

Wouldn't naming the entire flow 'ci' and the jobs as linting/testing be more suitable for opening up the workflow for future job additions?

- run: npm ci

- run: npm run lint
- run: npm run test

@ngkhengyang ngkhengyang Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it would be better to have testing and linting run as separate jobs so npm run test still runs even if linting fails. It's also easier to see which step passed/failed.

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