-
-
Notifications
You must be signed in to change notification settings - Fork 66
feat(add): drizzle add-on can now create multiple configs
#704
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.
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
Conversation
🦋 Changeset detectedLatest commit: fdf52b8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
|
Ohh, missed the PR. Why not override the existing files? I think the other addons are doing this as well, arent they? |
|
I think that is you add drizzle in a project with already drizzle you probably want to add another flavor. |
|
I think this would be really inconsistent with the other addons that we support. That's why i find it hard debating on it, even if I totally understand your point of view. |
|
The problem is that we have both valid points 😁 In all other add-ons, it would be strange to create another config/setup. So if files exists we try to augment them with the add-on. eg1: To write this I'm looking at the code and find that it's not fully consistent as well. For example, in vitest, if there is already The closer case to All in all, I think that it's quite "case by case", or "what make sense". In ORMs, I already had to deal with multiple dbs, that's why I think that it could make sense But also, tbf, it's not in majority of my projects! Here, there are 2 options:
In the 2 options, we have to fix the |
|
We could say something like "we've detected an existing config file. Would you like to update the existing one, skip, or create a new one?" |
|
Update an existing one is not really an option IMO. If you already have a schema, it would not make a lot of sense to change to ours. Maybe, to be closer to today, we could just craft a better "next step". So, not create a new db, but explain what to change to be able to relaunch cli to create a new db ? This PR is doing
|
|
I think it's just such an uncommon thing to run the adder a second time, that it's very hard to predict what the user is trying to accomplish. Some of the other adders try to fix the existing setup, but it seems so error prone and impossible to predict in what way things might be broken that I often think we'd be better off not even trying. I guess here there is some legitimate reason to add a second database, so I'm not opposed to it. It's pretty uncommon, so I don't know that I would have invested the effort, but I don't see any harm in it either
They might not want to if the existing one is their main db. In that case calling it db is fine. The second one might be some secondary db. I think it'd be fine to either give it a temporary name or ask them what to name it. If we asked what to call it then it would give us a way to update the existing one of they picked the same name as the existing one |
|
I see:
That's this PR :D |
|
After gathering feedback from several people (including some IRL chats 😄), it looks like this feature isn’t quite as 50/50 as I initially thought. So I’ll go ahead and close this one in favor of the simpler fix: #738 Thanks everyone for inputs! 🙏 |
Closes: #701
When it's the case, I added a line in
Next stepsLet me know what do you think.