|
| 1 | +# Contributing to Forklift Console Plugin |
| 2 | + |
| 3 | +Thank you for your interest in contributing to the Forklift Console Plugin. This guide covers the conventions, processes, and standards the team follows. |
| 4 | + |
| 5 | +For initial setup (clone, install, run), see [README.md](README.md#quick-start). |
| 6 | + |
| 7 | +## Branching Strategy |
| 8 | + |
| 9 | +Development happens on `main`. Release branches follow the pattern `release-X.Y` (e.g., `release-2.11`, `release-2.10`). |
| 10 | + |
| 11 | +- **Feature branches**: fork from `main`, open a PR against `main` |
| 12 | +- **Release branches**: cut from `main` when a release stabilizes |
| 13 | +- **Backports**: after a PR merges to `main`, comment `/backport release-X.Y` on the PR to create a cherry-pick PR to that release branch. `/cherrypick` and `/cherry-pick` also work. Add `--dry-run` to test first. |
| 14 | + |
| 15 | +## Pull Request Process |
| 16 | + |
| 17 | +1. **Fork and branch** -- fork the repo, create a feature branch from `main` |
| 18 | +2. **Keep PRs focused** -- one logical change per PR. Smaller PRs get faster reviews. |
| 19 | +3. **Ensure CI passes** -- every PR runs linters (ESLint, Stylelint, Knip), build, unit tests (Jest with coverage via Codecov), and upstream E2E tests (Playwright) |
| 20 | +4. **Request review** -- PRs require at least one approval from a project member |
| 21 | +5. **Squash merge** -- PRs are squash-merged into `main` |
| 22 | + |
| 23 | +### CI Checks on Every PR |
| 24 | + |
| 25 | +| Check | What it runs | |
| 26 | +|---|---| |
| 27 | +| Linters | `npm run lint`, `npm run test:i18n`, `npm run knip` | |
| 28 | +| Build | `npm run build` | |
| 29 | +| Tests + Coverage | `npm run test:coverage` (uploaded to Codecov) | |
| 30 | +| E2E | Playwright upstream tests against a KinD cluster | |
| 31 | +| Commit validation | Validates `Resolves:` line format in commit messages | |
| 32 | + |
| 33 | +## Commit Conventions |
| 34 | + |
| 35 | +All commits must be **signed off** (`git commit -s`) for DCO compliance. The DCO check fails without the `Signed-off-by` line. |
| 36 | + |
| 37 | +Every commit message must include a `Resolves:` line in the body. For the full format specification, examples, and troubleshooting, see [COMMIT_MESSAGE_GUIDE.md](COMMIT_MESSAGE_GUIDE.md). |
| 38 | + |
| 39 | +Quick reference: |
| 40 | + |
| 41 | +```text |
| 42 | +Fix provider validation for vSphere |
| 43 | +
|
| 44 | +Updated certificate handling for edge cases. |
| 45 | +
|
| 46 | +Resolves: MTV-456 |
| 47 | +Signed-off-by: Your Name <your-email@redhat.com> |
| 48 | +``` |
| 49 | + |
| 50 | +Validate locally before pushing: |
| 51 | + |
| 52 | +```bash |
| 53 | +npm run validate-commits |
| 54 | +``` |
| 55 | + |
| 56 | +## Coding Standards |
| 57 | + |
| 58 | +The project follows strict conventions documented in [AGENTS.md](AGENTS.md). Here is a summary of the most important rules: |
| 59 | + |
| 60 | +### TypeScript |
| 61 | + |
| 62 | +- **Strict mode** is enabled -- always handle nullable values |
| 63 | +- Use `type` instead of `interface` for type definitions |
| 64 | +- Use inline `type` keyword for mixed imports: `import { type FC, useState } from 'react'` |
| 65 | +- Never use `any` -- use `unknown` and narrow the type |
| 66 | +- Always define explicit return types on functions |
| 67 | +- Use `??` instead of `||` for default values |
| 68 | + |
| 69 | +### React |
| 70 | + |
| 71 | +- Never use default or star imports for React (`import React from 'react'` is wrong) |
| 72 | +- Use `FC` type for functional components |
| 73 | +- One component per file, default export at end of file |
| 74 | +- Extract logic into custom hooks or utility files -- keep components lean |
| 75 | +- Use `testId` prop (not `dataTestId`) for test identifiers |
| 76 | + |
| 77 | +### PatternFly |
| 78 | + |
| 79 | +- Use PatternFly 6 components |
| 80 | +- Use enum variants (`ButtonVariant.primary`) instead of string literals (`"primary"`) |
| 81 | +- Use the custom `Select` component from `@components/common/Select`, not PatternFly's directly |
| 82 | + |
| 83 | +### File Organization |
| 84 | + |
| 85 | +- Components: `.tsx` extension, PascalCase filename (e.g., `MyComponent.tsx`) |
| 86 | +- Folders: lowercase kebab-case (e.g., `my-component/`) |
| 87 | +- No barrel files (`index.ts`) -- import directly from source files |
| 88 | +- Path aliases: `@utils/*`, `@components/*`, `@test-utils/*`, `src/*` |
| 89 | +- Target file size: under 150 lines, hard limit 300 lines |
| 90 | + |
| 91 | +### Styling |
| 92 | + |
| 93 | +- Prefer SCSS with BEM methodology |
| 94 | +- Extract styles into separate `.scss` files |
| 95 | +- Use project-based class names, not PatternFly class names (they change between versions) |
| 96 | +- No `!important` |
| 97 | + |
| 98 | +## Internationalization |
| 99 | + |
| 100 | +All user-facing strings must be translatable. The project uses `react-i18next` with namespace `plugin__forklift-console-plugin`. |
| 101 | + |
| 102 | +- In components: `useForkliftTranslation` hook |
| 103 | +- For JSX interpolation: `<ForkliftTrans>` component |
| 104 | +- Outside components: `t` function from `src/utils/i18n` |
| 105 | + |
| 106 | +After adding new strings, run `npm run i18n` to extract translation keys. |
| 107 | + |
| 108 | +## Testing |
| 109 | + |
| 110 | +### Unit Tests (Jest) |
| 111 | + |
| 112 | +- Test files: `*.test.ts` or `*.test.tsx` |
| 113 | +- Use `@testing-library/react` for component tests |
| 114 | +- Run: `npm test` or `npm run test:coverage` |
| 115 | + |
| 116 | +### E2E Tests (Playwright) |
| 117 | + |
| 118 | +- Located in `testing/playwright/e2e/` |
| 119 | +- Page objects in `testing/playwright/page-objects/` |
| 120 | +- Upstream tests run in CI against a KinD cluster |
| 121 | +- Downstream tests run against real OpenShift clusters with providers |
| 122 | +- Run: `npm run test:e2e` |
| 123 | + |
| 124 | +## Linting |
| 125 | + |
| 126 | +ESLint and Prettier run automatically via Husky pre-commit hooks (`lint-staged`). Do not skip them. |
| 127 | + |
| 128 | +Key ESLint rules: |
| 129 | + |
| 130 | +- No `console.log` -- remove before committing |
| 131 | +- Objects must have alphabetically sorted keys |
| 132 | +- Exhaustive deps for `useEffect`, `useMemo`, `useCallback` |
| 133 | +- Imports auto-sorted by ESLint |
| 134 | + |
| 135 | +```bash |
| 136 | +npm run lint # check for issues |
| 137 | +npm run lint:fix # auto-fix |
| 138 | +``` |
| 139 | + |
| 140 | +## Getting Help |
| 141 | + |
| 142 | +- **Jira project**: MTV (tickets are `MTV-XXXX`) |
| 143 | +- **Repository**: [kubev2v/forklift-console-plugin](https://github.com/kubev2v/forklift-console-plugin) |
| 144 | +- **Forklift ecosystem**: [kubev2v/forklift](https://github.com/kubev2v/forklift) (operator and controllers) |
0 commit comments