Skip to content

Mutable config #4

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.

Sign up for GitHub

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

Jump to bottom
Merged
sunkan merged 3 commits into from
May 19, 2025
Merged

Mutable config #4

sunkan merged 3 commits into from
May 19, 2025

Conversation

@sunkan
Copy link
Member

@sunkan sunkan commented May 19, 2025

This is to make it easier to override config values at runtime

Mainly created to make it easier to change configs in tests

sunkan added 2 commits May 19, 2025 11:29
@sunkan
[config] Refactor tests to have common base
b6f0c7c 
This is to make sure different config implementation have the same
base tests to make sure they behave the same
@sunkan
[config] Add new MutableConfig
466722e 
This is to make it easier to override config values at runtime
Mainly created to make it easier to change configs in tests
@sunkan sunkan requested a review from bix0r May 19, 2025 09:36
bix0r
bix0r requested changes May 19, 2025
View reviewed changes
Copy link
Member

@bix0r bix0r left a comment

Choose a reason for hiding this comment

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

@sunkan Can you add some info about this in the README?
Perhaps this needs a note about not touching the "root" config. So that no one will think that a set will touch that part.
Otherwise it looks good.

@sunkan sunkan requested a review from bix0r May 19, 2025 12:11
bix0r
bix0r requested changes May 19, 2025
View reviewed changes
Copy link
Member

@bix0r bix0r left a comment

Choose a reason for hiding this comment

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

@sunkan Very nice. Love it! Just two missing characters :)

README.md Outdated
```php
<?php

// common.ph
Copy link
Member

@bix0r bix0r May 19, 2025

Choose a reason for hiding this comment

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

".php"

README.md Outdated
```php
<?php

// production.ph
Copy link
Member

@bix0r bix0r May 19, 2025

Choose a reason for hiding this comment

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

".php"

@sunkan
Copy link
Member Author

sunkan commented May 19, 2025

@bix0r you missed one there where 3 instances 😄

@sunkan sunkan requested a review from bix0r May 19, 2025 12:41
@sunkan sunkan merged commit ae55413 into main May 19, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bix0r bix0r bix0r approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sunkan @bix0r