feat: add comment policy automation via uncomment tool

- Add multi-platform uncomment tool integration via Nix flake
- Configure treefmt to enforce AAA-only comment policy
- Remove all explanatory comments from codebase
- Preserve only Arrange/Act/Assert test comments
This commit is contained in:
Venkatesan Ravi
2025-11-22 19:14:20 -08:00
parent 5d55e409ef
commit 392a12feba
15 changed files with 152 additions and 98 deletions

7
.uncommentrc.toml Normal file
View File

@@ -0,0 +1,7 @@
[global]
remove_todos = false
remove_fixme = false
remove_docs = false
preserve_patterns = ["Arrange", "Act", "Assert", "IMPORTANT", "NOTE", "WARNING", "HACK", "TODO", "FIXME"]
use_default_ignores = true
respect_gitignore = true

View File

@@ -31,7 +31,7 @@
perSystem = {system, ...}: let
pkgs = nixpkgs.legacyPackages.${system};
treefmtEval = treefmt-nix.lib.evalModule pkgs ./treefmt.nix;
treefmtEval = treefmt-nix.lib.evalModule pkgs ./nix/formatting/treefmt.nix;
in {
checks =
{

View File

@@ -1,51 +1,74 @@
_: {
{
lib,
pkgs,
system,
...
}: {
projectRootFile = "flake.nix";
programs = {
deadnix = {
enable = true;
priority = 10;
};
statix = {
enable = true;
priority = 20;
};
alejandra = {
enable = true;
priority = 30;
};
prettier = {
enable = true;
priority = 40;
settings.editorconfig = true;
};
deno = {
enable = true;
includes = [
"*.md"
];
priority = 50;
};
isort = {
enable = true;
includes = [
"*.py"
];
priority = 60;
profile = "black";
};
black = {
enable = true;
includes = [
"*.py"
];
priority = 80;
};
deadnix = {
enable = true;
priority = 10;
};
deno = {
enable = true;
includes = [
"*.md"
];
priority = 60;
};
isort = {
enable = true;
includes = [
"*.py"
];
priority = 70;
profile = "black";
};
prettier = {
enable = true;
priority = 50;
settings.editorconfig = true;
};
statix = {
enable = true;
priority = 20;
};
};
settings.formatter = {
"uncomment" = {
command = let
uncomment = import ./uncomment.nix {
inherit lib pkgs;
inherit (pkgs.stdenv.hostPlatform) system;
};
in "${uncomment}/bin/uncomment";
includes = [
"src/**/*.ts"
"src/**/*.js"
"tests/**/*.ts"
"tests/**/*.js"
];
priority = 40;
};
};
}

View File

@@ -0,0 +1,61 @@
{
lib,
pkgs,
system,
}: let
systemMap = {
"aarch64-darwin" = {
assetSuffix = "aarch64-apple-darwin";
sha256 = "sha256-Tpl7r1+f45rBQ3Kk3qWChhe2ZkdMxjwUodOqXjpAHLA=";
};
"aarch64-linux" = {
assetSuffix = "aarch64-unknown-linux-gnu";
sha256 = "sha256-7CJZ6osUg7k3HgdPDmBu9Jgyt5hjPkIMgBK9kd9s04c=";
};
"x86_64-darwin" = {
assetSuffix = "x86_64-apple-darwin";
sha256 = "sha256-RCsUhGRN9VRqygKLWhRv64RIDI/oVBtu5m7W5GJ9KBc=";
};
"x86_64-linux" = {
assetSuffix = "x86_64-unknown-linux-gnu";
sha256 = "sha256-mwXwExrvJZ7UkgRMbvagkANLyInvAYmHmiPmhePm6/0=";
};
};
info = systemMap.${system};
url = "https://github.com/Goldziher/uncomment/releases/download/v2.8.1/uncomment-${info.assetSuffix}.tar.gz";
in
pkgs.stdenv.mkDerivation {
buildInputs =
lib.optionals pkgs.stdenv.isLinux [pkgs.stdenv.cc.cc.lib];
installPhase = ''
runHook preInstall
mkdir -p $out/bin
cp uncomment $out/bin/uncomment
runHook postInstall
'';
meta = with pkgs.lib; {
description = "Tree-sitter based comment removal tool";
homepage = "https://github.com/Goldziher/uncomment";
license = licenses.mit;
mainProgram = "uncomment";
platforms = builtins.attrNames systemMap;
};
nativeBuildInputs =
lib.optionals pkgs.stdenv.isLinux [pkgs.autoPatchelfHook];
pname = "uncomment";
sourceRoot = ".";
src = pkgs.fetchurl {
inherit url;
inherit (info) sha256;
};
version = "2.8.1";
}

View File

@@ -38,7 +38,6 @@ function hasHardwareDecodingCodecsChanged(
const cur: string[] = current.HardwareDecodingCodecs ?? [];
const next: string[] = desired.hardwareDecodingCodecs;
// Order-independent comparison using deepEqual with sorted arrays
return !deepEqual([...cur].sort(), [...next].sort());
}

View File

@@ -23,7 +23,6 @@ export function calculateLibraryDiff(
const virtualFoldersToCreate: VirtualFolderConfig[] = [];
let hasUpdates: boolean = false;
// Check each desired virtual folder
for (const desiredVF of desired.virtualFolders) {
const existingVF: VirtualFolderInfoSchema | undefined =
currentVirtualFolders.find(
@@ -33,7 +32,6 @@ export function calculateLibraryDiff(
if (!existingVF) {
virtualFoldersToCreate.push(desiredVF);
} else {
// Check if locations differ
const currentLocations: string[] = existingVF.Locations ?? [];
const desiredLocations: string[] = desiredVF.libraryOptions.pathInfos.map(
(p: { path: string }) => p.path,
@@ -67,7 +65,6 @@ export async function applyLibrary(
return;
}
// Create missing virtual folders
for (const virtualFolder of libraryConfig.virtualFolders) {
logger.info(`Creating virtual folder: ${virtualFolder.name}`);

View File

@@ -31,7 +31,6 @@ function arePluginRepositoryConfigsEqual(
a: PluginRepositoryConfig[],
b: PluginRepositoryConfig[],
): boolean {
// Order-independent comparison using deepEqual with sorted arrays
const sortByNameAndUrl: (
repos: PluginRepositoryConfig[],
) => PluginRepositoryConfig[] = (

View File

@@ -50,7 +50,6 @@ export async function runPipeline(path: string): Promise<void> {
apiKey,
);
// Handle system configuration
const currentServerConfigurationSchema: ServerConfigurationSchema =
await jellyfinClient.getSystemConfiguration();
@@ -69,7 +68,6 @@ export async function runPipeline(path: string): Promise<void> {
console.log("✓ system config already up to date");
}
// Handle encoding configuration if provided
if (cfg.encoding) {
const currentEncodingOptionsSchema: EncodingOptionsSchema =
await jellyfinClient.getEncodingConfiguration();
@@ -86,7 +84,6 @@ export async function runPipeline(path: string): Promise<void> {
}
}
// Handle library configuration if provided
if (cfg.library) {
const currentVirtualFolders: VirtualFolderInfoSchema[] =
await jellyfinClient.getVirtualFolders();
@@ -102,7 +99,6 @@ export async function runPipeline(path: string): Promise<void> {
}
}
// branding configuration
if (cfg.branding) {
const currentBrandingSchema: BrandingOptionsDtoSchema =
await jellyfinClient.getBrandingConfiguration();
@@ -119,7 +115,6 @@ export async function runPipeline(path: string): Promise<void> {
}
}
// user management
if (cfg.users) {
let currentUsers: UserDtoSchema[] = await jellyfinClient.getUsers();

View File

@@ -61,7 +61,6 @@ import { type EncodingOptionsConfig } from "../../src/types/config/encoding-opti
import { type EncodingOptionsSchema } from "../../src/types/schema/encoding-options";
import * as loggerModule from "../../src/lib/logger";
// Mock the logger
vi.mock("../../src/lib/logger", () => ({
logger: {
info: vi.fn(),
@@ -96,8 +95,8 @@ describe("apply/encoding", () => {
// Assert
expect(result?.EnableHardwareEncoding).toBe(true);
expect(result?.EncodingThreadCount).toBe(-1); // Should preserve other fields
expect(result?.TranscodingTempPath).toBe("/tmp"); // Should preserve other fields
expect(result?.EncodingThreadCount).toBe(-1);
expect(result?.TranscodingTempPath).toBe("/tmp");
});
it("should update EnableHardwareEncoding when enableHardwareEncoding changes from true to false", () => {
@@ -119,7 +118,7 @@ describe("apply/encoding", () => {
// Assert
expect(result?.EnableHardwareEncoding).toBe(false);
expect(result?.EncodingThreadCount).toBe(4); // Should preserve other fields
expect(result?.EncodingThreadCount).toBe(4);
});
it("should not modify EnableHardwareEncoding when enableHardwareEncoding is undefined", () => {
@@ -280,7 +279,7 @@ describe("apply/encoding", () => {
// Assert
expect(result?.HardwareAccelerationType).toBe(expectedValue);
expect(result?.EncodingThreadCount).toBe(-1); // Should preserve other fields
expect(result?.EncodingThreadCount).toBe(-1);
},
);
});
@@ -457,9 +456,8 @@ describe("apply/encoding", () => {
// Assert
expect(result?.EnableHardwareEncoding).toBe(true);
expect(result?.HardwareAccelerationType).toBe("nvenc");
expect(result?.EncodingThreadCount).toBe(4); // Should preserve other fields
expect(result?.EncodingThreadCount).toBe(4);
// Should log both changes
expect(loggerSpy).toHaveBeenCalledWith(
"EnableHardwareEncoding changed: false → true",
);
@@ -477,8 +475,8 @@ describe("apply/encoding", () => {
} as EncodingOptionsSchema;
const desired: EncodingOptionsConfig = {
enableHardwareEncoding: true, // Same value
hardwareAccelerationType: "videotoolbox", // Different value
enableHardwareEncoding: true,
hardwareAccelerationType: "videotoolbox",
};
const loggerSpy: Mock<(msg: string) => void> = vi.spyOn(
@@ -496,14 +494,12 @@ describe("apply/encoding", () => {
expect(result?.EnableHardwareEncoding).toBe(true);
expect(result?.HardwareAccelerationType).toBe("videotoolbox");
// Should only log the change
expect(loggerSpy).toHaveBeenCalledWith(
"HardwareAccelerationType changed: vaapi → videotoolbox",
);
expect(loggerSpy).toHaveBeenCalledTimes(1);
});
// Device Fields Tests (vaapiDevice, qsvDevice)
describe("device fields", () => {
it("should update VaapiDevice when vaapiDevice changes", () => {
// Arrange
@@ -560,7 +556,7 @@ describe("apply/encoding", () => {
// Assert
expect(result?.VaapiDevice).toBe(expectedValue);
expect(result?.EncodingThreadCount).toBe(2); // Should preserve other fields
expect(result?.EncodingThreadCount).toBe(2);
},
);
});
@@ -620,7 +616,7 @@ describe("apply/encoding", () => {
// Assert
expect(result?.QsvDevice).toBe(expectedValue);
expect(result?.EncodingThreadCount).toBe(3); // Should preserve other fields
expect(result?.EncodingThreadCount).toBe(3);
},
);
});
@@ -717,7 +713,6 @@ describe("apply/encoding", () => {
});
});
// Array Field Tests (hardwareDecodingCodecs)
describe("hardwareDecodingCodecs array field", () => {
it("should update HardwareDecodingCodecs when hardwareDecodingCodecs changes", () => {
// Arrange
@@ -790,7 +785,7 @@ describe("apply/encoding", () => {
// Assert
expect(result?.HardwareDecodingCodecs).toEqual(expectedValue);
expect(result?.EncodingThreadCount).toBe(2); // Should preserve other fields
expect(result?.EncodingThreadCount).toBe(2);
},
);
});
@@ -860,7 +855,6 @@ describe("apply/encoding", () => {
});
});
// Boolean Decoding Fields Tests
describe("boolean decoding fields", () => {
describe("enableDecodingColorDepth10Hevc", () => {
it("should update EnableDecodingColorDepth10Hevc when enableDecodingColorDepth10Hevc changes", () => {
@@ -1144,7 +1138,6 @@ describe("apply/encoding", () => {
});
});
// Boolean Encoding Format Fields Tests
describe("boolean encoding format fields", () => {
describe("allowHevcEncoding", () => {
it("should update AllowHevcEncoding when allowHevcEncoding changes", () => {
@@ -1311,7 +1304,6 @@ describe("apply/encoding", () => {
});
});
// Complete 11-field scenario test
it("should handle all 11 encoding options fields together in complete scenario", () => {
// Arrange
const current: EncodingOptionsSchema = {
@@ -1371,9 +1363,8 @@ describe("apply/encoding", () => {
expect(result?.EnableDecodingColorDepth12HevcRext).toBe(false);
expect(result?.AllowHevcEncoding).toBe(false);
expect(result?.AllowAv1Encoding).toBe(false);
expect(result?.EncodingThreadCount).toBe(8); // Should preserve other fields
expect(result?.EncodingThreadCount).toBe(8);
// Should log changes for fields that actually changed
expect(loggerSpy).toHaveBeenCalledWith(
"EnableHardwareEncoding changed: false → true",
);
@@ -1393,7 +1384,6 @@ describe("apply/encoding", () => {
"EnableDecodingColorDepth10HevcRext changed: false → true",
);
// Should not log for fields that didn't change (same values)
expect(loggerSpy).not.toHaveBeenCalledWith(
expect.stringContaining("QsvDevice changed"),
);
@@ -1410,7 +1400,6 @@ describe("apply/encoding", () => {
expect.stringContaining("AllowAv1Encoding changed"),
);
// Total calls should be 6 (only the fields that actually changed)
expect(loggerSpy).toHaveBeenCalledTimes(6);
});
});

View File

@@ -38,7 +38,6 @@ import type { ServerConfigurationSchema } from "../../src/types/schema/system";
import type { SystemConfig } from "../../src/types/config/system";
import * as loggerModule from "../../src/lib/logger";
// Mock the logger
vi.mock("../../src/lib/logger", () => ({
logger: {
info: vi.fn(),
@@ -595,7 +594,7 @@ describe("apply/system", () => {
// Assert
expect(result?.TrickplayOptions?.EnableHwAcceleration).toBe(true);
expect(result?.TrickplayOptions?.EnableHwEncoding).toBe(true); // Should preserve
expect(result?.TrickplayOptions?.EnableHwEncoding).toBe(true);
expect(result?.EnableMetrics).toBe(false);
expect(result?.PluginRepositories).toEqual([]);
});
@@ -620,7 +619,7 @@ describe("apply/system", () => {
calculateSystemDiff(current, desired);
// Assert
expect(result?.TrickplayOptions?.EnableHwAcceleration).toBe(true); // Should preserve
expect(result?.TrickplayOptions?.EnableHwAcceleration).toBe(true);
expect(result?.TrickplayOptions?.EnableHwEncoding).toBe(true);
expect(result?.EnableMetrics).toBe(false);
expect(result?.PluginRepositories).toEqual([]);
@@ -743,8 +742,8 @@ describe("apply/system", () => {
const desired: SystemConfig = {
trickplayOptions: {
enableHwAcceleration: true, // Same
enableHwEncoding: true, // Different
enableHwAcceleration: true,
enableHwEncoding: true,
},
};
@@ -887,7 +886,7 @@ describe("apply/system", () => {
expect(result?.PluginRepositories).toEqual([
{ Name: "New", Url: "https://new.com", Enabled: false },
]);
expect(result?.TrickplayOptions).toEqual(existingTrickplay); // Preserved
expect(result?.TrickplayOptions).toEqual(existingTrickplay);
});
it("should update two fields while preserving the third (enableMetrics + trickplayOptions)", () => {
@@ -919,15 +918,15 @@ describe("apply/system", () => {
// Assert
expect(result?.EnableMetrics).toBe(true);
expect(result?.PluginRepositories).toEqual(existingRepos); // Preserved
expect(result?.PluginRepositories).toEqual(existingRepos);
expect(result?.TrickplayOptions?.EnableHwAcceleration).toBe(true);
expect(result?.TrickplayOptions?.EnableHwEncoding).toBe(false); // Preserved within object
expect(result?.TrickplayOptions?.EnableHwEncoding).toBe(false);
});
it("should update two fields while preserving the third (pluginRepositories + trickplayOptions)", () => {
// Arrange
const current: ServerConfigurationSchema = {
EnableMetrics: true, // Will be preserved
EnableMetrics: true,
PluginRepositories: [
{ Name: "Old", Url: "https://old.com", Enabled: true },
],
@@ -951,11 +950,11 @@ describe("apply/system", () => {
calculateSystemDiff(current, desired);
// Assert
expect(result?.EnableMetrics).toBe(true); // Preserved
expect(result?.EnableMetrics).toBe(true);
expect(result?.PluginRepositories).toEqual([
{ Name: "New", Url: "https://new.com", Enabled: false },
]);
expect(result?.TrickplayOptions?.EnableHwAcceleration).toBe(false); // Preserved within object
expect(result?.TrickplayOptions?.EnableHwAcceleration).toBe(false);
expect(result?.TrickplayOptions?.EnableHwEncoding).toBe(true);
});

View File

@@ -9,10 +9,8 @@ describe("lib/deep-equality", () => {
const obj2: { c: number; b: number; a: number } = { c: 3, b: 2, a: 1 };
// Act & Assert
// deep equality should correctly identify these as equal
expect(deepEqual(obj1, obj2)).toBe(true);
// JSON.stringify would incorrectly identify these as different
expect(JSON.stringify(obj1) === JSON.stringify(obj2)).toBe(false);
});
@@ -22,16 +20,13 @@ describe("lib/deep-equality", () => {
const obj2: { a: number; b: undefined } = { a: 1, b: undefined };
// Act & Assert
// deep equality should correctly handle undefined
expect(deepEqual(obj1, obj2)).toBe(true);
// JSON.stringify would not serialize undefined values at all
expect(JSON.stringify(obj1) === JSON.stringify(obj2)).toBe(true);
// But JSON.stringify would miss differences involving undefined
const obj3: { a: number } = { a: 1 };
expect(deepEqual(obj1, obj3)).toBe(false);
expect(JSON.stringify(obj1) === JSON.stringify(obj3)).toBe(true); // false positive!
expect(JSON.stringify(obj1) === JSON.stringify(obj3)).toBe(true);
});
it("should handle nested objects correctly", () => {
@@ -64,14 +59,13 @@ describe("lib/deep-equality", () => {
// Act & Assert
expect(deepEqual(obj1, obj2)).toBe(true);
expect(JSON.stringify(obj1) === JSON.stringify(obj2)).toBe(true); // works but converts to string
expect(JSON.stringify(obj1) === JSON.stringify(obj2)).toBe(true);
// But deepEqual preserves type information
const obj3: { timestamp: string } = {
timestamp: "2024-01-01T00:00:00.000Z",
};
expect(deepEqual(obj1, obj3)).toBe(false); // correctly different types
expect(JSON.stringify(obj1) === JSON.stringify(obj3)).toBe(true); // false positive!
expect(deepEqual(obj1, obj3)).toBe(false);
expect(JSON.stringify(obj1) === JSON.stringify(obj3)).toBe(true);
});
});
});

View File

@@ -117,11 +117,9 @@ describe("EncodingOptionsConfig", () => {
// Assert
expect(result.success).toBe(false);
if (!result.success) {
// Should have an invalid_enum_value error for hardwareAccelerationType
expect(result.error.issues).toBeDefined();
expect(result.error.issues.length).toBeGreaterThan(0);
// Find the error related to hardwareAccelerationType
const hasEnumError: boolean = result.error.issues.some(
(issue: z.core.$ZodIssue) =>
issue.code === "invalid_value" &&

View File

@@ -169,7 +169,6 @@ describe("types/config/library", () => {
},
};
// Type assertions to verify correct inference
expect(typeof parsed.name).toBe("string");
expect(parsed.collectionType).toMatch(
/^(movies|tvshows|music|musicvideos|homevideos|boxsets|books|mixed)$/,
@@ -248,7 +247,7 @@ describe("types/config/library", () => {
},
},
{
name: "", // Invalid - empty name
name: "",
collectionType: "tvshows",
libraryOptions: {
pathInfos: [{ path: "/test" }],
@@ -292,9 +291,7 @@ describe("types/config/library", () => {
virtualFolders: [
// @ts-expect-error intentional invalid object for test
{
// Missing required fields
name: "Test",
// Missing collectionType and libraryOptions
},
],
};
@@ -304,7 +301,6 @@ describe("types/config/library", () => {
});
it("should properly type virtualFolders as optional", () => {
// Type test - should compile without errors
const config1: LibraryConfig = {};
const config2: LibraryConfig = { virtualFolders: undefined };
const config3: LibraryConfig = { virtualFolders: [] };
@@ -320,7 +316,6 @@ describe("types/config/library", () => {
],
};
// Runtime assertions
expect(config1.virtualFolders).toBeUndefined();
expect(config2.virtualFolders).toBeUndefined();
expect(config3.virtualFolders).toEqual([]);

View File

@@ -204,7 +204,6 @@ describe("RootConfig", () => {
// Arrange
const invalidConfig: Partial<z.input<typeof RootConfigType>> = {
version: 1,
// missing base_url and system
};
// Act

View File

@@ -152,7 +152,6 @@ describe("PluginRepositoryConfig", () => {
// Arrange
const invalidConfig: Partial<z.input<typeof PluginRepositoryConfigType>> = {
name: "Test Repository",
// missing url and enabled
};
// Act