Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 4m53s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m12s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 5m7s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m34s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 5m4s
Should_Not_Crash_Mid_Roll_{Zero,One}_Chapters passed 'new PlayoutItem()'
to AddFiller, whose Start defaults to DateTime.MinValue. The mid-roll Pad
path (PlayoutModeSchedulerBase.cs:609) subtracts 'currentMinute' minutes
from StartOffset; since StartOffset does ToLocalTime(), a non-zero local
offset makes currentMinute non-zero, underflowing DateTimeOffset.MinValue
and throwing ArgumentOutOfRangeException. Under UTC the offset is 0 so it
stays in range (hence CI was green). Unreachable in production — real
playout items never start at MinValue — so this is a test-data artifact,
not a product bug.
Fix: seed the fixtures with a real Start (startState.CurrentTime.UtcDateTime),
matching the convention in the passing AddFiller tests. Verified across
UTC, Europe/Brussels, and Asia/Kolkata (+5:30); full Core.Tests now passes
under any timezone (484/0). Updated docs/contributing.md §8 per its §10
deviation policy.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
144 lines
9.1 KiB
Markdown
144 lines
9.1 KiB
Markdown
# Contributing to the ErsatzTV fork
|
|
|
|
A **descriptive** guide to the conventions already in this codebase — written so future changes
|
|
(human or AI) extend the established architecture instead of reinventing it. It's a living
|
|
document: update it when a pattern legitimately evolves.
|
|
|
|
## The guiding principle
|
|
|
|
**Match the established style and patterns. Diverge only when there's a genuinely strong reason —
|
|
when it's the real best or only option — even if that means rearchitecting. Never deviate
|
|
gratuitously or just because a different approach is locally convenient. When a deviation is
|
|
justified, state why** (in the PR/commit, and in code comments where it'll surprise a reader).
|
|
|
|
Several of the rules below are now **enforced in CI** (layering tests, analyzers, formatting,
|
|
migration checks) — see the pointers per section.
|
|
|
|
## 1. Architecture & layering
|
|
|
|
CQRS via MediatR. Dependency direction is enforced by `ErsatzTV.Architecture.Tests` (NetArchTest):
|
|
|
|
| Project | Responsibility | May depend on |
|
|
|---|---|---|
|
|
| `ErsatzTV.FFmpeg` | ffmpeg process/pipeline wrapper | (nothing — lowest layer) |
|
|
| `ErsatzTV.Core` | domain entities, interfaces, pure logic | `FFmpeg` only — **no** EF Core, **no** Infrastructure/Application |
|
|
| `ErsatzTV.Infrastructure` | EF Core data access, external clients | `Core` |
|
|
| `ErsatzTV.Infrastructure.Sqlite` / `.MySql` | provider-specific EF (migrations) | `Core`, `Infrastructure` |
|
|
| `ErsatzTV.Application` | MediatR handlers (business logic) | `Core`, `Infrastructure` (the **abstraction**, not the concrete providers) |
|
|
| `ErsatzTV.Scanner` | library scanning host | `Core`, `Infrastructure*` |
|
|
| `ErsatzTV` | ASP.NET Core host, Blazor, controllers, DI composition root | everything |
|
|
|
|
If a test fails here, the fix is almost always to move the type, not to relax the rule. (ersatztv#12)
|
|
|
|
## 2. CQRS handler conventions
|
|
|
|
- Requests are **`record` types** named for the action — `GetX` / `CreateX` / `UpdateX` / `DeleteX` —
|
|
implementing `IRequest<TResult>`, living under `<Domain>/Queries/` or `<Domain>/Commands/`
|
|
**next to** their handler.
|
|
- **Result types** (LanguageExt, see §3): queries return `Option<T>` or `List<T>`; commands return
|
|
`Option<BaseError>` (void success) or `Either<BaseError, T>` (value success).
|
|
- Handlers implement `IRequestHandler<TRequest, TResult>` with **primary-constructor DI** and an
|
|
`async Task<TResult> Handle(TRequest request, CancellationToken cancellationToken)`.
|
|
- **Validation lives in the handler** as private static methods returning `Validation<BaseError, T>`,
|
|
composed with tuple `.Apply(...)` — **not** FluentValidation or MediatR pipeline behaviors.
|
|
- Examples: `ErsatzTV.Application/Channels/Queries/GetChannelById.cs` (+ `…/GetChannelByIdHandler.cs`,
|
|
`Option<ChannelViewModel>`), `ErsatzTV.Application/FFmpegProfiles/Commands/DeleteFFmpegProfileHandler.cs`
|
|
(tuple-`Apply` validation → `Either<BaseError, Unit>`).
|
|
|
|
## 3. Functional style (LanguageExt)
|
|
|
|
Every project globally imports `using static LanguageExt.Prelude;`. Use the monads instead of
|
|
exceptions for control flow.
|
|
|
|
- **`Option<T>`** — `Optional(x)`, `Some`/`None`, `HeadOrNone()` to pluck from a collection;
|
|
branch with `Match`/`MatchAsync(onSome, onNone)`; default with `IfNone(...)`; transform with
|
|
`Map`/`Bind`; `foreach (var x in option)` to unpack conditionally.
|
|
- **`Either<BaseError, T>` / `Validation<BaseError, T>`** — the error backbone. `BaseError.New("…")`
|
|
for failures, `Unit.Default` for void success. Accumulate multiple checks with `Validation` (tuple
|
|
`.Apply((a, b, c) => …)`) then `ToEither()`. `MapLeft` to transform errors.
|
|
- Examples: `ErsatzTV.Core/FFmpeg/…`, `ErsatzTV.Application/FFmpegProfiles/Commands/DeleteFFmpegProfileHandler.cs`.
|
|
|
|
## 4. Blazor / MudBlazor UI
|
|
|
|
- **Keep pages thin** — inject `IMediator` and `await Mediator.Send(new SomeQuery(...), token)`;
|
|
no business logic or data access in the component.
|
|
- Pages `@implements IDisposable` and own a `CancellationTokenSource` per lifecycle phase, cancelled
|
|
on parameter change / disposal.
|
|
- Forms: `MudForm` + **Blazored FluentValidation** (`Validation="@(validator.ValidateValue)"`,
|
|
`For="@(() => _model.Prop)"`); surface errors via `Snackbar.Add(msg, Severity.Error)`.
|
|
- Dialogs: pass `DialogParameters`, accept `[CascadingParameter] IMudDialogInstance`, close with
|
|
`MudDialog.Close(DialogResult.Ok(...))`. Global providers + event subscriptions live in
|
|
`Shared/MainLayout.razor` (unsubscribed in `Dispose()`).
|
|
- Examples: `ErsatzTV/Pages/Channels.razor`, `ErsatzTV/Pages/ChannelEditor.razor`,
|
|
`ErsatzTV/Shared/ChannelPreviewDialog.razor`.
|
|
|
|
## 5. Data access & EF Core
|
|
|
|
- `TvContext` (`ErsatzTV.Infrastructure/Data/TvContext.cs`) is the single DbContext; resolve it via
|
|
`IDbContextFactory<TvContext>` in handlers.
|
|
- **Two providers**: `ErsatzTV.Infrastructure.Sqlite` (the prod default, `/config/ersatztv.sqlite3`)
|
|
and `…MySql`, each with its own `Migrations/` set + `TvContextModelSnapshot`. The provider is
|
|
chosen by the `provider` config key in `Startup`.
|
|
- **A model change needs a migration in BOTH providers** — run `scripts/add-migration.sh <Name>`.
|
|
CI's `migrations` job enforces model-drift + apply-to-fresh-DB per provider (see `docs/ci-cd.md`
|
|
→ Migration integrity). Review non-transactional ops (e.g. SQLite `PRAGMA foreign_keys`) carefully.
|
|
(ersatztv#13)
|
|
|
|
## 6. FFmpeg pipeline
|
|
|
|
- **Step-based composition**: each task is an `IPipelineStep` (`ErsatzTV.FFmpeg/IPipelineStep.cs`) that
|
|
contributes `GlobalOptions`/`InputOptions`/`FilterOptions`/`OutputOptions` and a `NextState()` that
|
|
threads frame metadata (pixel format, data location, dimensions). `CommandGenerator.GenerateArguments`
|
|
assembles the steps into the final argument list.
|
|
- **Detect capabilities at runtime; don't hardcode** — `IHardwareCapabilities` implementations
|
|
(`Vaapi`/`Nvidia`/… `HardwareCapabilities`) report available codecs/profiles, and builders pick the
|
|
encoder/decoder accordingly.
|
|
- **Version-gate features explicitly** — e.g. `Capabilities/NvidiaHardwareCapabilities.cs` parses
|
|
`FFmpegCapabilities.Version` and disables 10-bit H.264 decode hwaccel below ffmpeg 8. Follow this
|
|
pattern for the ffmpeg 8 upgrade (ersatztv#9).
|
|
- Examples: `ErsatzTV.FFmpeg/Encoder/EncoderBase.cs`, `ErsatzTV.FFmpeg/Filter/ScaleFilter.cs`.
|
|
|
|
## 7. Naming, formatting, analyzers
|
|
|
|
- **`.editorconfig` is the source of truth** for formatting + rule severities (plus
|
|
`ErsatzTV.sln.DotSettings` for ReSharper). Run `dotnet format` before committing.
|
|
- **`TreatWarningsAsErrors=true`** in the app projects — a warning fails the build. `NoWarn` carries a
|
|
small, documented exemption list (e.g. `VSTHRD200`, `CA1873`); NuGet-audit `NU1901-1903` are demoted
|
|
to warnings in `Directory.Build.props` while `NU1904` (critical) blocks.
|
|
- **Static-analysis packs** (Roslynator, SonarAnalyzer, Meziantou, AsyncFixer) run at `suggestion` and
|
|
are promoted to `warning`/`error` rule-by-rule; promotion is the enforcement (the TWAE build). New
|
|
rules start at suggestion — never flip a wall of rules to error at once. (ersatztv#15)
|
|
|
|
## 8. Testing
|
|
|
|
- **NUnit** (`[TestFixture]`/`[Test]`/`[TestCase]`) + **Shouldly** (`.ShouldBe(...)`) + **NSubstitute**
|
|
+ Testably.Abstractions for a fake filesystem. **xUnit is not used.** Tests live in `*.Tests`
|
|
projects mirroring the source.
|
|
- Established kinds: **FFmpeg command-string assertions** (build a pipeline → assert the exact arg
|
|
string — `ErsatzTV.FFmpeg.Tests/PipelineBuilderBaseTests.cs`); **golden-file tests** for
|
|
Jellyfin-facing output (`ErsatzTV.Core.Tests/Iptv/ChannelPlaylistGoldenTests.cs`, regen via
|
|
`ETV_UPDATE_GOLDENS=1`, ersatztv#11); **architecture tests** (§1, ersatztv#12).
|
|
- The suite is **timezone-independent** — the previously timezone-sensitive `DateTimeOffset`
|
|
filler-scheduling tests were fixed (ersatztv#24) by giving fixtures realistic times instead of the
|
|
default `DateTime.MinValue`, which underflowed `DateTimeOffset.MinValue` under a non-UTC offset.
|
|
When constructing test `PlayoutItem`s, set a real `Start` (e.g. `startState.CurrentTime.UtcDateTime`),
|
|
not the default. CI runs UTC and uses `dotnet test … --blame-hang-timeout 2m`.
|
|
|
|
## 9. Build / CI
|
|
|
|
- **Central Package Management**: versions live in the repo-root `Directory.Packages.props`; csproj
|
|
reference packages by name only — never put `Version=` back on a `<PackageReference>` (NU1008).
|
|
Repo-wide MSBuild config is in `Directory.Build.props` / `Directory.Build.targets`. (ersatztv#14)
|
|
- The Gitea Actions pipeline (`.gitea/workflows/docker-build.yml`): `test` → `migrations` →
|
|
`build` (image + smoke/IPTV-E2E). **Renovate** opens dependency PRs; a weekly `dependency-scan`
|
|
surfaces advisories. Full details: **`docs/ci-cd.md`**.
|
|
- Versioning is CalVer `vYY.<release-seq>.<patch>` (not year.month); never `[skip ci]` a commit you'll
|
|
tag. See `docs/ci-cd.md` → Versioning.
|
|
|
|
## 10. Deviation policy
|
|
|
|
Re-stating §0 because it's the whole point: **follow the established pattern; diverge only with a
|
|
concrete, stated reason.** Record an intentional departure in the PR description and a code comment at
|
|
the point of surprise (e.g. "using X instead of the usual Y because …"). If a pattern itself should
|
|
change, change it deliberately and update this guide in the same PR.
|