Skip to content

Conversation

asgeija
Copy link
Member

@asgeija asgeija commented Oct 5, 2026

Runs lint, format check, tests and build on every pull request to main and on pushes to main, so broken code is caught before review.

Uses npm ci to match the lockfile, caches dependencies and cancels outdated runs to save runner time.

Refs #45

Runs lint, format check, tests and build on every pull request to
main and on pushes to main, so broken code is caught before review.

Uses npm ci to match the lockfile, caches dependencies and cancels outdated runs to save runner time.

Refs #45
@asgeija asgeija added this to the Partial submission 2 milestone Oct 5, 2026
@asgeija asgeija self-assigned this Oct 5, 2026
@asgeija asgeija requested a review from a team as a code owner October 5, 2026 10:47
Vitest exits with code 1 when it finds no tests, which made the CI
test step fail before any tests exist. Adds --passWithNoTests as a
temporary measure.

Refs #45
Adds format check and build steps so formatting and type errors are
caught before review. Limits triggers to pull requests and pushes to
main to avoid duplicate runs, caches npm dependencies and cancels
outdated runs to save runner time.

Refs #45
@thortve
Copy link
Member

thortve commented Oct 5, 2026

@asgeija is there now a branch ruleset with status check requirement?

@thortve
Copy link
Member

thortve commented Oct 5, 2026

@asgeija maybe look into changing the prettier format from a simple check to a more sophisticated solution where the runner automatically commits the prettier formatting. Removes one less tedious task from out backs, and also results in more status checks passed. Making the CI more streamlined.

@asgeija
Copy link
Member Author

asgeija commented Oct 5, 2026

@asgeija is there now a branch ruleset with status check requirement?

Not yet. If we want to, the status check can only be selected after it has run once,
so we can add the ruleset after this PR is merged and the first run
has passed. It will require the checks job on main.

@asgeija
Copy link
Member Author

asgeija commented Oct 5, 2026

@asgeija maybe look into changing the prettier format from a simple check to a more sophisticated solution where the runner automatically commits the prettier formatting. Removes one less tedious task from out backs, and also results in more status checks passed. Making the CI more streamlined.

Thanks, but I'd rather not have the runner commit. A new commit
dismisses approvals, and commits pushed with the default token don't
re-trigger the workflow, so the required check could go missing. We can
later add a .vscode/settings.json with format on save instead, and keep
format:check in CI as the safety net.

thortve
thortve previously approved these changes Oct 5, 2026
Applies the Prettier config to files that did not follow it, mainly
the ESLint and Vite configs, so the CI format check passes. Also
aligns markdown tables and spacing in AGENTS.md and CONTRIBUTING.md.
No behavior changes.
Runs Prettier on files that came in with the submit sighting form
and the unit tests, so the CI format check passes. No behavior
changes.
Unit tests now exist on main, so CI should fail if no tests are found
instead of passing silently.
The submit form (#46) made Banner use router links and useSighting
read from a store, which broke the tests added in #22. Wraps Banner
in a MemoryRouter, looks for the title as a link and runs useSighting
inside renderHook. No production code changes.
Copy link
Member

@vitaliyb vitaliyb left a comment

Choose a reason for hiding this comment

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

Good, ready to merge.

@vitaliyb vitaliyb merged commit 19ebe82 into main Oct 5, 2026
1 check passed
@asgeija asgeija deleted the asgeija-add-ci-workflow branch October 5, 2026 11:25
@asgeija
Copy link
Member Author

asgeija commented Oct 5, 2026

@asgeija is there now a branch ruleset with status check requirement?

There is now a ruleset on main that requires the checks job

Sign in to join this conversation on GitHub.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants