Skip to content
New issue

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

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

Already on GitHub? # to your account

feat: Ability to change reload strategy from AutoMapper::create() #183

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

MrMeshok
Copy link
Contributor

@MrMeshok MrMeshok commented Sep 4, 2024

No description provided.

@MrMeshok MrMeshok force-pushed the reload-strategy-from-create branch from 3fce8e6 to 9b8064f Compare September 4, 2024 07:00
@joelwurtz
Copy link
Member

joelwurtz commented Sep 4, 2024

Thanks for this PR, Could this use the Configuration class instead of being a parameter, i think we should go this way for every configuration parameter, it will be easier for BC break rather than having a new parameter

However you may need to change the symfony bundle in order to make it work (not sure)

@MrMeshok
Copy link
Contributor Author

MrMeshok commented Sep 4, 2024

Yea, I thought about putting it in Configuration, but create method already has cacheDirectory argument, so it seems natural to put reloadStrategy there too. Also, I did have a look in the symfony bundle, as far as I can tell, AutoMapper creates from DI without using create method, and reloadStrategy from FileLoader gets replaced through container

Copy link
Member

@Korbeil Korbeil 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 this feature @MrMeshok, could you move the parameter to the Configuration class and add a note about your change in the CHANGELOG please ? 🙏

@MrMeshok MrMeshok force-pushed the reload-strategy-from-create branch from 9b8064f to 2fa514a Compare September 16, 2024 09:30
@MrMeshok
Copy link
Contributor Author

@Korbeil Done. I also did a small change in Symfony bundle, although it is not necessary

@MrMeshok MrMeshok force-pushed the reload-strategy-from-create branch from 2fa514a to b613607 Compare September 16, 2024 09:34
@Korbeil Korbeil merged commit 9de87a7 into jolicode:main Sep 16, 2024
5 checks passed
@Korbeil
Copy link
Member

Korbeil commented Sep 16, 2024

Thanks for your contribution @MrMeshok !

@MrMeshok MrMeshok deleted the reload-strategy-from-create branch September 16, 2024 10:34
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants