Development #2
Development #2
Conversation
Added the configuration page under the "Plugins" bar to the left in Jellyfin settings. Made so settings are more easily added in the future. Code is a lot more cleaner and transparent also separated into a html and js file.
Bif files are now regenerated if the configuration does not match, removed option to have multiple bif files, might implement it back in the future. Added some more logging when I have time I will add more.
Added option to not regenerate manifest files / bif files not made by Jellyscrubberr
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @CasperDoesCoding - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 6 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| }; | ||
|
|
||
| // If a Manifest object already exists, combine resolutions | ||
| // Check if a manifest already exists for this item and if it matches the current configuration |
There was a problem hiding this comment.
suggestion (code_refinement): Consider extracting manifest existence and match logic into a separate method for clarity.
| // Serialize and write to manifest file | ||
| using FileStream createStream = File.Create(path); | ||
| await JsonSerializer.SerializeAsync(createStream, newManifest); | ||
| await createStream.DisposeAsync(); |
There was a problem hiding this comment.
suggestion (code_refinement): Consider using 'await using' statement for automatic disposal of FileStream.
| catch (JsonException) | ||
| { | ||
| _logger.LogWarning("Error deserializing manifest file at {0}", path); | ||
| return true; // ? I dont know what causes this error so to prevent files being regenerated, return true for now. If anyone knows what causes this error, please let me know. |
There was a problem hiding this comment.
issue (code_clarification): Remove or address inline comment about unknown error causes before merging.
| @@ -37,6 +37,8 @@ public class TrickplayController : ControllerBase | |||
| private readonly IServerConfigurationManager _configurationManager; | |||
| private readonly EncodingHelper _encodingHelper; | |||
| private readonly PluginConfiguration _config; | |||
| private static ManifestManager _manifestManager = null!; | |||
There was a problem hiding this comment.
issue (code_refinement): Avoid using static fields for service instances to prevent potential lifecycle management issues.
| @@ -0,0 +1,178 @@ | |||
| const pluginId = "6b961c92-5678-45d1-a7ac-dd5003f03460"; | |||
There was a problem hiding this comment.
suggestion (code_refinement): Consider moving pluginId to a shared constants file if used across multiple files.
No description provided.