Skip to content

Migrate to the Options Pattern #240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 19, 2024
Merged

Migrate to the Options Pattern #240

merged 5 commits into from
Dec 19, 2024

Conversation

Sella-GH
Copy link
Owner

No description provided.

@Sella-GH Sella-GH added refactoring Stuff which is related to refactoring breaking-change Stuff which breaks the current configuration area-database Database related stuff area-bot Stuff which is related to the bot area-core Stuff which is related to the core library labels Dec 19, 2024
@Sella-GH Sella-GH added this to the 2.2.0 milestone Dec 19, 2024
@Sella-GH Sella-GH self-assigned this Dec 19, 2024
@Copilot Copilot AI review requested due to automatic review settings December 19, 2024 19:19
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 17 out of 32 changed files in this pull request and generated 1 comment.

Files not reviewed (15)
  • Directory.Build.props: Language not supported
  • src/AzzyBot.Bot/Modules/Core/Files/AppStats.json: Language not supported
  • src/AzzyBot.Bot/Settings/AzzyBotSettings-Docker.json: Language not supported
  • src/AzzyBot.Bot/Settings/AzzyBotSettings.json: Language not supported
  • src/AzzyBot.Bot/Settings/AzzyBotSettingsRecord.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Commands/Autocompletes/GuildsAutocomplete.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Commands/AdminCommands.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Commands/Autocompletes/AzzyHelpAutocomplete.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Services/DiscordBotService.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Services/DiscordEvents/DiscordGuildsHandler.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Commands/CoreCommands.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Services/UpdaterService.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Services/Modules/CoreServiceHost.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Services/DiscordBotServiceHost.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Extensions/IServiceCollectionExtensions.cs: Evaluated as low risk
Comments suppressed due to low confidence (3)

src/AzzyBot.Bot/Settings/Validators/AppStatsValidator.cs:7

  • The AppStatsValidator class should contain validation logic or at least a placeholder method to indicate that validation will be implemented.
public sealed partial class AppStatsValidator : IValidateOptions<AppStats>

src/AzzyBot.Bot/Settings/AzzyBotSettings.cs:49

  • [nitpick] The variable name 'LavalinkPassword' might be unclear. Consider renaming it to something more descriptive like 'MusicStreamingPassword'.
public string? LavalinkPassword { get; set; } = "AzzyB0TMus1cStr3am!ng";

src/AzzyBot.Bot/Settings/AzzyBotSettings.cs:31

  • [nitpick] The default value for 'Doing' might be too specific. Consider using a more generic default value like 'Idle'.
public string Doing { get; set; } = "Music";

@Sella-GH Sella-GH requested a review from Copilot December 19, 2024 19:44
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 17 out of 32 changed files in this pull request and generated 2 comments.

Files not reviewed (15)
  • Directory.Build.props: Language not supported
  • src/AzzyBot.Bot/Modules/Core/Files/AppStats.json: Language not supported
  • src/AzzyBot.Bot/Settings/AzzyBotSettings-Docker.json: Language not supported
  • src/AzzyBot.Bot/Settings/AzzyBotSettings.json: Language not supported
  • src/AzzyBot.Bot/Settings/AzzyBotSettingsRecord.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Settings/AzzyBotSettings.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Settings/AppSettingsRecord.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Settings/Validators/AppStatsValidator.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Services/DiscordEvents/DiscordGuildsHandler.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Services/UpdaterService.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Services/DiscordBotServiceHost.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Services/Modules/CoreServiceHost.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Commands/AdminCommands.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Commands/Autocompletes/GuildsAutocomplete.cs: Evaluated as low risk
  • src/AzzyBot.Bot/Commands/Autocompletes/AzzyHelpAutocomplete.cs: Evaluated as low risk
Comments suppressed due to low confidence (4)

src/AzzyBot.Bot/Settings/Validators/AzzyBotSettingsValidator.cs:8

  • The AppStatsValidator class is defined but does not contain any validation logic. Ensure that validation logic is implemented and covered by tests.
}

src/AzzyBot.Bot/Extensions/IServiceCollectionExtensions.cs:35

  • Building a service provider within the ConfigureServices method can cause issues with the DI container, such as creating multiple instances of services that should be singletons. Consider refactoring this to avoid building the service provider.
IServiceProvider serviceProvider = services.BuildServiceProvider();

src/AzzyBot.Bot/Extensions/IServiceCollectionExtensions.cs:104

  • The AddOptionsWithValidateOnStart method is not a standard method in the Microsoft.Extensions.DependencyInjection namespace. Ensure that this method is defined and correctly implemented, or replace it with the standard ValidateOnStart method.
services.AddSingleton<IValidateOptions<AzzyBotSettings>, AzzyBotSettingsValidator>().AddOptionsWithValidateOnStart<AzzyBotSettings>();

src/AzzyBot.Bot/Commands/CoreCommands.cs:113

  • [nitpick] The class name CoreStats should be renamed to match the convention of using a noun or noun phrase, such as CoreStatistics, to improve clarity.
public sealed class CoreStats(AppStats stats, ILogger<CoreStats> logger)

@Sella-GH Sella-GH merged commit c1a6a43 into main Dec 19, 2024
1 check passed
@Sella-GH Sella-GH deleted the dev/OptionsPattern branch December 19, 2024 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-bot Stuff which is related to the bot area-core Stuff which is related to the core library area-database Database related stuff breaking-change Stuff which breaks the current configuration refactoring Stuff which is related to refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant