feat: POC for ddev config --update #6053
Conversation
|
WOW! |
|
Download the artifacts for this pull request:
See Testing a PR. |
| apptypeMsg := fmt.Sprintf("Apptype is %s", app.Type) | ||
| output.UserOut.Print(apptypeMsg) | ||
|
|
||
| if app.Type == nodeps.AppTypeDrupal { |
There was a problem hiding this comment.
I don't think you should need to mention specific project types here.
There was a problem hiding this comment.
Yeah, it was only for debugging purposes when I did that. I just thought that we might want to take this chance to change apptype drupal8, drupal9, drupal10 to drupal if we find those.
|
I think the fundamental difference is shown in Essentially we need to do what it was doing before that... but only on update? |
Oh right I was accidentally hit by that one! Makes sense, it would be probably cleaner. One question I have if we go either path is: should we really change db? That might be dangerous. Even if I don't think data loss could happen, people might not find their data if the db versions change. I suggest if a change in db is found, we leave it as is and provide instructions on updating/migrating it. |
| if appFuncs, ok := appTypeMatrix[app.Type]; ok && appFuncs.configOverrideAction != nil && !app.ConfigExists() { | ||
| // of config.yaml that it needs to | ||
| func (app *DdevApp) ConfigFileOverrideAction(overrideExistingConfig bool) error { | ||
| if appFuncs, ok := appTypeMatrix[app.Type]; ok && appFuncs.configOverrideAction != nil && (overrideExistingConfig || !app.ConfigExists()) { |
There was a problem hiding this comment.
@rfay Even if we go the other path, we probably still need this.
We should not change database type or version. It would make people crazy. Thanks for paying attention to this. I see that this would affect CraftCMS, which really wants MySQL 8.0. But I don't see why we'd change existing. I see drupal8/9 have override to mariadb:10.4, but I don't think those belong in there in the first place? Why would we not use defaults? Silverstripe is probably a mistake. @GuySartorelli should we be overriding database type and version in ddev/pkg/ddevapp/silverstripe.go Lines 78 to 79 in 64c9e46 |
|
Closing just to avoid confusion, can be reopened. |
|
I recognise this was closed but just answering what was asked:
I'm a bit removed from the workflow of someone who uses Silverstripe CMS to make websites - my workflow is very specific to the maintenance work I do so I'm not sure whether this would be a useful or harmful. It is worth noting that there have been a few discussions about how DDEV overrides values in |
The Issue
How This PR Solves The Issue
For supporting discussion on #6035.
If we end up going this path I'd create a cleaner PR.
Manual Testing Instructions
Automated Testing Overview
Related Issue Link(s)
Release/Deployment Notes