- 🇩🇪Germany Anybody Porta Westfalica
I like this idea and I'm maintainer now. Could someone prepare a MR for review?
- First commit to issue fork.
- @roshni27 opened merge request.
- Status changed to Needs review
over 1 year ago 11:30am 27 July 2023 - 🇮🇳India roshni27
Hello ,
I have applied patch #3 on 8.x-1.x and 2.x but it does not apply cleanly.
I have implement the language code by referencing #3 and #5 comment.
Please review the MR.
If any mistake please guide me.
Thanks - Status changed to RTBC
over 1 year ago 12:33pm 19 September 2023 - Status changed to Needs work
over 1 year ago 1:33pm 19 September 2023 - 🇧🇬Bulgaria pfrenssen Sofia
Thanks for the great work on this!
I had a look at the merge request. It looks good, but we are adding a new option to the configuration, so this also needs a config schema update and a simple post-update hook to set the default value for existing installations.
- Status changed to Needs review
over 1 year ago 2:00pm 19 September 2023 - 🇧🇬Bulgaria pfrenssen Sofia
Updated the config schema and provided an update hooks, and did some minor cleanup. I have not tested my changes.
- 🇩🇪Germany Anybody Porta Westfalica
Thanks all, looking really good!
@pfrenssen I might be wrong, but shouldn't "simple" config updates like this use regular update hooks?
See https://www.specbee.com/blogs/update-and-post-update-hooks-for-successfu... - 🇧🇬Bulgaria pfrenssen Sofia
I'm always confused about this. According to the documentation, config updates are supported in
hook_update_N()
, but I always thought it is best practice to only use those for data model updates nowadays. I saw a bunch of config updates in the post-update hooks of the core system module so I went with that. E.g.system_post_update_service_advisory_settings()
I guess both work. If you want I can change it.
- 🇩🇪Germany Anybody Porta Westfalica
@pfrenssen all good, I'm totally fine with that and I'm also confused, what's the right way ... we can keep it like this.
So only needs final testing?