Skip to content

Conversation

Murasko
Copy link
Contributor

@Murasko Murasko commented Sep 19, 2025

Adds serde_yaml dependency and removes the lum_config FileHandler.

@Murasko Murasko self-assigned this Sep 19, 2025
@Murasko Murasko requested review from Copilot and Kitt3120 September 19, 2025 09:06
Copy link

@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.

Pull Request Overview

This PR replaces the lum_config FileHandler with direct YAML configuration loading using serde_yaml. The change simplifies configuration management by removing a dependency layer and implementing custom YAML file handling logic.

Key changes:

  • Removes FileHandler dependency from lum_config and adds serde_yaml dependency
  • Implements custom config file loading with automatic directory creation and default config generation
  • Adds example YAML configuration file showing the expected format

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/main.rs Replaces FileHandler with custom YAML config loading logic and adds new error variants
example-config.yaml Provides example configuration showing YAML structure and available options
Cargo.toml Adds serde_yaml dependency

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@Kitt3120 Kitt3120 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. I made some recommendations. We should also think about adding yaml support in lum_config upstream instead of implementing it here. But that's up to the future, implementing it here is okay for now :)

@Copilot Copilot AI review requested due to automatic review settings September 20, 2025 12:06
Copy link

@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.

Pull Request Overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Murasko Murasko requested a review from Kitt3120 September 20, 2025 13:33
@Copilot Copilot AI review requested due to automatic review settings September 21, 2025 18:43
@Kitt3120 Kitt3120 force-pushed the feat/json-to-yaml-config branch from 3b2aa3a to 0db9675 Compare September 21, 2025 18:43
Copy link

@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.

Pull Request Overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Kitt3120 Kitt3120 enabled auto-merge (squash) September 21, 2025 18:44
Kitt3120
Kitt3120 previously approved these changes Sep 21, 2025
@Kitt3120 Kitt3120 disabled auto-merge September 21, 2025 18:49
@Kitt3120 Kitt3120 merged commit 84dfddc into main Sep 21, 2025
3 checks passed
@Kitt3120 Kitt3120 deleted the feat/json-to-yaml-config branch September 21, 2025 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants