Skip to content

CONTRIBUTING.md

Latest commit

 

History

History
253 lines (176 loc) · 9.74 KB

CONTRIBUTING.md

File metadata and controls

253 lines (176 loc) · 9.74 KB
Raw

Contributing to TeachLink Mobile

Thank you for contributing to TeachLink Mobile!

Pull Request Guidelines

When submitting a Pull Request, you must fill out the provided PR template.
The template ensures that all necessary considerations are accounted for before merge.

Please review the .github/pull_request_template.md which includes:

  • Summary & Type of Change: Describe what the PR does.
  • Testing Done: List the tests performed.
  • Security Considerations: Address concerns like secure data storage, token handling, input validation, and deep link handling.
  • Performance Considerations: Address concerns like hook optimization (useCallback, useMemo), FlatList optimization, and asynchronous patterns.
  • Checklist: General checks, including checking whether an Architectural Decision Record (ADR) is needed.

Fast-Fail Syntax Gate

We have a dedicated Syntax Gate workflow (.github/workflows/syntax.yml) that runs on every pull request opened or synchronize event.

  • Checks TypeScript compiler errors (tsc --noEmit) and ESLint (eslint --max-warnings=0)
  • Optimized to complete in under 90 seconds using caching
  • Required for branch protection — PRs cannot be merged if it fails
  • Run checks locally before pushing to avoid CI failures

Architecture

The intended module structure, layering and dependency direction are documented in docs/ARCHITECTURE.md. The layering is enforced locally and in CI with dependency-cruiser:

npm run architecture:check

Read the architecture doc before adding a new module — the codebase already has a single canonical implementation for error handling, logging, location, course progress, sync conflict resolution, and feature flags, and duplicating one of these is a review blocker.

Structured Logging

Never use console.* in src/. The ESLint no-console rule is set to error, and CI will fail if any console.* call is introduced. Use src/utils/logger instead.

Why structured logging?

console.log output is unstructured, always-on, and leaks information in production builds. logger gives you:

  • Log level filtering (only error and warn in production)
  • Consistent metadata (timestamp, component context)
  • A single place to redirect logs to remote monitoring (e.g. Sentry, Datadog)

Log level guide

Level Method When to use
error logger.error(msg, err?) Unexpected failures that need immediate attention. Always include the Error object as the second argument.
warn logger.warn(msg, ctx?) Recoverable issues or deprecated code paths that should be investigated.
info logger.info(msg, ctx?) Key lifecycle events: component mount/unmount, navigation, background sync. Keep them meaningful, not noisy.
debug logger.debug(msg, ctx?) Verbose detail useful during development only. Stripped from production builds.
component logger.component(name, event, ctx?) Convenience wrapper for component lifecycle events — equivalent to info with a standardised format.

Examples

// ✅ Correct
import { logger } from '../../utils/logger';

logger.component('MyScreen', 'Mounted', { userId });
logger.info('Resuming lesson from position:', position);
logger.warn('Quiz data missing for section:', sectionId);
logger.error('Failed to sync progress:', error);

// ❌ Incorrect — will fail CI
console.log('user mounted', userId);
console.error('sync failed', error);

Audit

CI runs a console violation scan on every push. To run it locally:

grep -rn "console\." src/ --include='*.ts' --include='*.tsx'

Zero matches is the expected output.

Local Quality Checks

You can run the checks locally:

# Run ESLint linting
npm run lint

# Check formatting
npm run format:check

# Run TypeScript type check (same check CI runs)
npm run typecheck

# Continuously re-run the type check as you edit
npm run typecheck:watch

Lint warning budget

Lint warnings are capped by a single ratcheting budget in lint-budget.json (maxWarnings), enforced by ci.yml. There is exactly one lint gate in CI.

  • npm run lint:budget — fails if the current warning count exceeds the budget, or if the budget is looser than the measured count (so the ceiling can only decrease over time).
  • npm run lint:budget:record — measures the warning count and records it as the new, lower budget. Run and commit this after removing warnings so the budget ratchets down instead of silently growing.

Git hooks

Husky hooks enforce a baseline before changes reach CI:

  • pre-commit — runs lint-staged (Prettier + ESLint) on staged files.
  • pre-push — runs npm run typecheck so type errors are caught before push.

You can bypass the hooks for a one-off push with git push --no-verify, but note that the same checks still run in CI and will block the pull request.

Contributing to TeachLink Mobile

Thank you for contributing to TeachLink Mobile!

Pull Request Guidelines

When submitting a Pull Request, you must fill out the provided PR template.
The template ensures that all necessary considerations are accounted for before merge.

Please review the .github/pull_request_template.md which includes:

  • Summary & Type of Change: Describe what the PR does.
  • Testing Done: List the tests performed.
  • Security Considerations: Address concerns like secure data storage, token handling, input validation, and deep link handling.
  • Performance Considerations: Address concerns like hook optimization (useCallback, useMemo), FlatList optimization, and asynchronous patterns.
  • Checklist: General checks, including checking whether an Architectural Decision Record (ADR) is needed.

Fast-Fail Syntax Gate

We have a dedicated Syntax Gate workflow (.github/workflows/syntax.yml) that runs on every pull request opened or synchronize event.

  • Checks TypeScript compiler errors (tsc --noEmit) and ESLint (eslint --max-warnings=0)
  • Optimized to complete in under 90 seconds using caching
  • Required for branch protection — PRs cannot be merged if it fails
  • Run checks locally before pushing to avoid CI failures

Architecture

The intended module structure, layering and dependency direction are documented in docs/ARCHITECTURE.md. The layering is enforced locally and in CI with dependency-cruiser:

npm run architecture:check

Read the architecture doc before adding a new module — the codebase already has a single canonical implementation for error handling, logging, location, course progress, sync conflict resolution, and feature flags, and duplicating one of these is a review blocker.

Structured Logging

Never use console.* in src/. The ESLint no-console rule is set to error, and CI will fail if any console.* call is introduced. Use src/utils/logger instead.

Why structured logging?

console.log output is unstructured, always-on, and leaks information in production builds. logger gives you:

  • Log level filtering (only error and warn in production)
  • Consistent metadata (timestamp, component context)
  • A single place to redirect logs to remote monitoring (e.g. Sentry, Datadog)

Log level guide

Level Method When to use
error logger.error(msg, err?) Unexpected failures that need immediate attention. Always include the Error object as the second argument.
warn logger.warn(msg, ctx?) Recoverable issues or deprecated code paths that should be investigated.
info logger.info(msg, ctx?) Key lifecycle events: component mount/unmount, navigation, background sync. Keep them meaningful, not noisy.
debug logger.debug(msg, ctx?) Verbose detail useful during development only. Stripped from production builds.
component logger.component(name, event, ctx?) Convenience wrapper for component lifecycle events — equivalent to info with a standardised format.

Examples

// ✅ Correct
import { logger } from '../../utils/logger';

logger.component('MyScreen', 'Mounted', { userId });
logger.info('Resuming lesson from position:', position);
logger.warn('Quiz data missing for section:', sectionId);
logger.error('Failed to sync progress:', error);

// ❌ Incorrect — will fail CI
console.log('user mounted', userId);
console.error('sync failed', error);

Audit

CI runs a console violation scan on every push. To run it locally:

grep -rn "console\." src/ --include='*.ts' --include='*.tsx'

Zero matches is the expected output.

Local Quality Checks

You can run the checks locally:

# Run ESLint linting
npm run lint

# Check formatting
npm run format:check

# Run TypeScript type check (same check CI runs)
npm run typecheck

# Continuously re-run the type check as you edit
npm run typecheck:watch

Lint warning budget

Lint warnings are capped by a single ratcheting budget in lint-budget.json (maxWarnings), enforced by ci.yml. There is exactly one lint gate in CI.

  • npm run lint:budget — fails if the current warning count exceeds the budget, or if the budget is looser than the measured count (so the ceiling can only decrease over time).
  • npm run lint:budget:record — measures the warning count and records it as the new, lower budget. Run and commit this after removing warnings so the budget ratchets down instead of silently growing.

Git hooks

Husky hooks enforce a baseline before changes reach CI:

  • pre-commit — runs lint-staged (Prettier + ESLint) on staged files.
  • pre-push — runs npm run typecheck so type errors are caught before push.

You can bypass the hooks for a one-off push with git push --no-verify, but note that the same checks still run in CI and will block the pull request.