Files
ersatztv/docs/contributing.md
T
timothyandClaude Opus 4.8 fe4cd39070
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
fix: make filler scheduling tests timezone-independent (fixes #24)
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>
2026-06-27 14:22:44 +02:00

9.1 KiB

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 hardcodeIHardwareCapabilities 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 PlayoutItems, 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): testmigrationsbuild (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.