Mediatools - #19
Merged
Merged
Conversation
Signed-off-by: Pieter Viljoen <ptr727@users.noreply.github.com>
Signed-off-by: Pieter Viljoen <ptr727@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR appears to be a significant refactoring of the ProjectTemplate repository titled "Mediatools", transitioning the library from Serilog-based logging to Microsoft.Extensions.Logging abstractions. The changes include introducing a new CodeGen project for automated code generation, restructuring the Console application, simplifying the Library to use generic logging abstractions, and updating documentation and GitHub Actions workflows.
Changes:
- Introduced new CodeGen project with automated quote-of-the-day code generation and GitHub Actions workflow integration
- Refactored Library to use Microsoft.Extensions.Logging instead of Serilog directly, improving portability and AOT compatibility
- Updated Console application to bridge Serilog and Microsoft.Extensions.Logging, removed unused functionality (ProcessTask, command-line options)
- Restructured README.md with extensive documentation improvements and new sections for installation, configuration, and usage
- Consolidated and renamed GitHub Actions workflows for better organization and added automated merge capabilities for bot-generated PRs
Reviewed changes
Copilot reviewed 47 out of 48 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/Tests.csproj | Removed AssemblyName and NeutralLanguage properties; added IsPackable and IsTestProject properties |
| Library/Library.csproj | Removed Serilog and CLI dependencies; added Microsoft.Extensions.Logging.Abstractions; moved AOT verification to conditional property group |
| Library/Library.cs | New file introducing Options and TemplateLibrary classes using Microsoft.Extensions.Logging |
| Library/GlobalUsings.cs | Updated global usings to use Microsoft.Extensions.Logging instead of Serilog |
| Library/Extensions.cs | Updated logging extensions to use LoggerMessage source generators with Microsoft.Extensions.Logging |
| Console/Console.csproj | Removed InternalsVisibleTo; added Serilog.Extensions.Logging; moved AOT properties to conditional group |
| Console/Program.cs | Refactored using primary constructor; removed helper methods; added library initialization (though unused) |
| Console/CommandLine.cs | Removed path, threads, and dryrun command-line options; simplified to logging options only |
| Console/ProcessTask.cs | Deleted file - removed all file processing functionality |
| Console/LoggerFactory.cs | Added CreateLogger method to bridge Serilog to Microsoft.Extensions.Logging |
| Console/Extensions.cs | Added extension methods for both Serilog and Microsoft.Extensions.Logging ILogger types |
| CodeGen/CodeGen.csproj | New project file for code generation executable |
| CodeGen/Program.cs | New code generation application that fetches quote-of-the-day and generates C# code |
| CodeGen/CommandLine.cs | Command-line parsing for codepath and apikey options |
| CodeGen/HttpClientFactory.cs | HTTP client factory with resilience policies for API calls |
| Docker/Dockerfile | Removed AOT build dependencies (clang, zlib1g-dev) |
| Docker/Build.sh | Removed AOT build steps |
| README.md | Extensive restructuring with new sections, badges, installation guide, and usage examples |
| HISTORY.md | New file documenting release history |
| .github/workflows/* | Multiple workflow files renamed, consolidated, and new codegen workflows added |
| ProjectTemplate.slnx | Added CodeGen project; removed Console build dependency from Tests |
| .vscode/launch.json | Added CodeGen launch configuration |
| .editorconfig files | Added suppressions for CA1515 in Console and CodeGen projects |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 47 out of 48 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (3)
.github/workflows/build-release-task.yml:64
- Inconsistent naming convention for workflow step names. Most steps now follow the pattern "name step" (e.g., "Setup .NET SDK step", "Checkout code step"), but this one says "Download library build artifacts job". The word "job" should be "step" for consistency with the other steps in this workflow.
.github/workflows/build-release-task.yml:70 - Inconsistent naming convention for workflow step names. Most steps now follow the pattern "name step" (e.g., "Setup .NET SDK step", "Checkout code step"), but this one says "Download executable build artifacts job". The word "job" should be "step" for consistency with the other steps in this workflow.
.github/workflows/build-release-task.yml:76 - Inconsistent naming convention for workflow step names. Most steps now follow the pattern "name step" (e.g., "Setup .NET SDK step", "Checkout code step"), but this one says "Create GitHub release job". The word "job" should be "step" for consistency with the other steps in this workflow.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 47 out of 48 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
.github/workflows/build-release-task.yml:76
- The step names use "job" suffix instead of "step" suffix, which is inconsistent with the naming convention established in other workflow files where steps are named with "step" suffix (e.g., lines 61, 64, 70, 76). This creates confusion between jobs and steps in the workflow.
This was referenced Jul 16, 2026
Closed
ptr727
added a commit
that referenced
this pull request
Jul 17, 2026
Fixes #332 (core gap + the six bundled nits from the #330 sweep reviews; two of the six live in the blessed downstream reference and propagate via the Vantage-Config carry, not this repo - see below). ## configure.sh: model inference on downstream carries In a carry the registry is absent, so the no-arg form defaulted to `release` and then hit the intended fail-safe abort on operational carries (missing `develop.json`) - arg 2 was effectively mandatory there. Now the model is inferred from which `develop` payload is carried (a carry holds exactly its own model's); ambiguous layouts (both or neither - e.g. a partial copy) abort rather than guess. Also: the model may be passed as the sole argument (`configure.sh operational`) - previously arg 1 was always parsed as a repo name (bundled nit, Utilities #417). Matrix-tested with the apply path stubbed: infer release, infer operational, both-present abort (exit 1), neither-present abort (exit 1), explicit arg-2 override, model-as-sole-arg, unknown-model abort. Hub behavior unchanged (registry present -> registry lookup, as before). shellcheck + bash -n clean. ## Ruleset snippet hardening (duplicate-name id corruption) The `id=$(gh api ... select(.name==...).id)` pattern in the README regen snippet and AUDIT.md section 6 produced a **multiline id** when duplicate same-name rulesets exist, silently corrupting the follow-up API path (bundled nit, Utilities #417 + LanguageTags #278). Both snippets now fetch the list once and enforce **exactly one** ruleset per name: the regen snippet fails loudly (never regen from a guess); the audit snippet reports `found N (defect/drift)` and continues - matching the declared drift semantics. Live-tested read-only against this repo (develop + main in sync; duplicate simulation counts correctly). ## README wording Documents the inference fallback and the model-only argument form (bundled nit, Utilities #417). ## Bundled nits NOT in this PR (with rationale) - `gh api --input -` portability (MediaTools #19): declined - fleet standard is current gh; not worth temp-file churn. - jq-derived secret name lists + `--paginate` in the secrets check (MediaTools #19, LanguageTags #278, PlexCleaner #854): these live in the **blessed downstream self-audit reference** (the Vantage-Config carry), not in hub files - propagated there directly after this merges (the hub's fleet audit reads `spec/secrets.json` programmatically already). ## Propagation caveat This re-drifts the five existing carries (Vantage-Config + the four #330 sweep repos) until their next conformance touch; Vantage-Config is updated immediately after this merges since it is the blessed reference the queued operational standups (HomeAssistant-Config #16, ESPHome-Config #46, HomeAutomation-Config #21) will adapt from. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.