Fix review findings F-311, F-291, F-475, F-473, F-293, F-468 - #589
Merged
Conversation
Addresses six low-severity findings from the review report. The LiveSessionData REST endpoint now returns a dedicated ViewData contract instead of exposing the internal ILiveSessionData interface. Program.cs documents its intentional use of Console output ahead of DI-configured logging. F1PacketTester now builds deterministically since it carries no wildcard-versioned assembly info. Dead EF Core package references were removed from F1SessionFolderRename, and an accidental Microsoft.NET.Test.Sdk reference was removed from F1Server.WebApi. The TimeTrial cell in MainWindow.xaml is realigned to match its sibling cells. Closes #227, closes #208, closes #375, closes #373, closes #209, closes #368
|
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
2 participants
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.
Pull Request
📖 Description
Fixes a batch of six low-severity findings from the repository review report (
docs/review/FINDINGS-OVERVIEW.md):LiveSessionDataController.Get()returned the internalILiveSessionDatainterface directly instead of a dedicated view data contract. AddedLiveSessionDataViewData/LiveDriverDataViewDatainF1Server.Data.ViewDataand mapped the runtime data to them, matching the ViewData pattern used elsewhere in the API.Program.csusesConsole.WriteLinethroughout instead ofILogger. Documented this as an intentional deviation: most of the startup flow runs before or independently of the DI-configured, opt-in OpenTelemetry logging pipeline.Deterministic=False(inherited fromDirectory.Build.props, needed solution-wide because of a wildcardAssemblyVersionin the linkedSharedAssemblyInfo.cs) was disabling deterministic builds forF1PacketTestereven though that project does not linkSharedAssemblyInfo.cs. OverrodeDeterministic=truelocally for that project.F1SessionFolderRename.csprojreferencedMicrosoft.EntityFrameworkCore.Designand anMicrosoft.EntityFrameworkCoreUpdateitem even though the tool does no EF work. Removed the dead configuration.F1Server.WebApi.csprojreferencedMicrosoft.NET.Test.Sdk, which belongs only inF1Server.Tests. Removed the accidental reference.F1ReplayClient/MainWindow.xamlto match its sibling cells.🎫 Issues
Closes #227, closes #208, closes #375, closes #373, closes #209, closes #368
👩💻 Reviewer Notes
Each change is independent and low-risk (DTO mapping, csproj cleanup, doc comment, XAML formatting). No behavior change is expected other than the
LiveSessionDataREST response now being a shaped view model (frontend contract fields were kept identical, includingdbId).📑 Test Plan
Full backend build and the existing
F1Server.Testssuite were run locally to confirm the changes do not introduce regressions. No new tests were needed since these are cleanup/formatting/DTO-mapping changes with no new logic branches.✅ Checklist
General
Backend-specific (.NET)
F1Server.*.F1Server.Testsfor the change.F1Server.Db.MsSqlMigrations,F1Server.Db.MySqlMigrationsandF1Server.Db.PostgreSqlMigrations.Frontend-specific (Angular)
⏭ Next Steps
None.