Skip to content
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

MariaDB mentions #5713

Merged
merged 4 commits into from
Mar 21, 2024
Merged

MariaDB mentions #5713

merged 4 commits into from
Mar 21, 2024

Conversation

ottok
Copy link
Contributor

@ottok ottok commented Mar 13, 2024

Describe this PR

I have never used Prisma before, and it took me quite a lot of extra effort to figure out if Prisma supports MariaDB at all because the documentation was only mentioning MySQL. After some digging, I found that indeed Prisma uses both https://www.npmjs.com/package/mariadb for the connector and https://hub.docker.com/_/mariadb as part of the CI, and MariaDB support is actually excellent.

What issue does this fix?

The docs hide the fact that Prisma actually supports MariaDB. Sequalize has very clear MariaDB documentation, and if you merge this PR then Prisma will be much more clear about the MariaDB support situation.

Changes

Each commit is atomic and carefully documented - please review this PR by looking commit-by-commit looking at the message and the change at the same time.

Looking at git log in 500-databases and e.g. 4358a41 seems you prefer to squash when merging. Please merge this PR as-is without squashing to avoid loosing the per-commit documentation.

Any other relevant information

I am also happy to iterate on the contents of this PR based on your feedback. I am also happy to rebase frequently.

Copy link

vercel bot commented Mar 13, 2024

@ottok is attempting to deploy a commit to the Prisma Team on Vercel.

A member of the Team first needs to authorize it.

@jharrell
Copy link
Member

Hi there @ottok , is this PR distinct from #5706 ?

@ottok ottok changed the title Mariadb mentions MariaDB mentions Mar 14, 2024
@ottok ottok force-pushed the mariadb-mentions branch from 46dfe51 to d59769f Compare March 14, 2024 02:31
@ottok
Copy link
Contributor Author

ottok commented Mar 14, 2024

Yes, I split this and also #5716 out from #5706 in hope that it is easier for you to review and to accept a few smaller PRs instead of one big one.

@jharrell
Copy link
Member

Hey there @ottok thanks again for your pull requests.

We want to pull some of this in, but not all of it. Would you mind changing this PR to only have the changes to these files?

  • content/200-orm/100-prisma-schema/20-data-model/20-relations/410-referential-actions/index.mdx
  • content/200-orm/500-reference/375-supported-databases.mdx
  • src/components/shortcodes/index.tsx

These are references where we think that, indeed, we also need to mention MariaDB. We can then review that and get it merged in.

As for #5706 , we can't accept it in its current state. MariaDB is supported via the MySQL provider, so having duplicated documentation that can quickly get out of sync isn't something we want to do.

Instead, we'd like to change the title of content/200-orm/050-overview/500-databases/400-mysql.mdx to MySQL and MariaDB from just MySQL. An additional sentence describing that we also support MariaDB due to it being compatible with MySQL would be nice as well.

I hope that all makes sense! Thank you again for your work.

@ottok ottok force-pushed the mariadb-mentions branch from 285767c to 5a08c8b Compare March 19, 2024 03:05
@ottok
Copy link
Contributor Author

ottok commented Mar 19, 2024

Thanks for the feedback! I have updated the PR following your guidance.

@ottok ottok force-pushed the mariadb-mentions branch 2 times, most recently from 0fb0116 to 53b6a3f Compare March 20, 2024 04:17
@ottok ottok requested a review from jharrell March 20, 2024 04:17
@ottok
Copy link
Contributor Author

ottok commented Mar 20, 2024

Thanks Jon for the feedback - I have updated the PR accordingly and also rebased on latest prisma:main.

Looking at git log in 500-databases and e.g. 4358a41 seems you prefer to squash when merging. Please merge this PR as-is without squashing to avoid loosing the per-commit documentation. Each commit is atomic and carefully documented.

Copy link

vercel bot commented Mar 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 21, 2024 1:06pm

@jharrell
Copy link
Member

Looking at git log in 500-databases and e.g. 4358a41 seems you prefer to squash when merging. Please merge this PR as-is without squashing to avoid loosing the per-commit documentation. Each commit is atomic and carefully documented.

@ottok is there any particular reason why this needs to be not squashed and merged? I appreciate the work you did here and am happy to merge, but we squash/merge as a default. Commits are still available in the respective PR.

@ottok
Copy link
Contributor Author

ottok commented Mar 21, 2024

Looking at git log in 500-databases and e.g. 4358a41 seems you prefer to squash when merging. Please merge this PR as-is without squashing to avoid loosing the per-commit documentation. Each commit is atomic and carefully documented.

@ottok is there any particular reason why this needs to be not squashed and merged? I appreciate the work you did here and am happy to merge, but we squash/merge as a default. Commits are still available in the respective PR.

If you care about linear history, or about being able to read git commits and the messages through tools like git blame etc, you should prefer linear history and normal merges or rebase+merge. But this is just a sidetrack for the doc change - git use is a topic for another day. Feel free to merge as you wish.

Thanks for the feedback rounds to get this polished, the result will now make the docs much better for MariaDB users. Thanks!

ottok added 3 commits March 20, 2024 22:23
For new users learning about Prisma it is unclear if Prisma properly
supports MariaDB or not. Mentioning MariaDB explicitly makes it easy
for users to find that Prisma does indeed support MariaDB.
When reading the Getting Started tutorial, FAQ and similar introductory
pages it is unclear if Prisma properly supports MariaDB or not.
Mentioning MariaDB explicitly makes it easy for users to find that
Prisma does indeed support MariaDB.
@ottok ottok force-pushed the mariadb-mentions branch from 53b6a3f to 90fdb9f Compare March 21, 2024 05:23
@jharrell jharrell merged commit 0211b13 into prisma:main Mar 21, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants