search

ocluf / justship

A svelte 5 and sveltekit saas boilerplate. Including database, authentication, email, analytics
https://justship.today
MIT License
456 stars 31 forks source link

fix: migration helper #8

Closed Creative-Difficulty closed 4 months ago

vercel[bot] commented 4 months ago

@Creative-Difficulty is attempting to deploy a commit to the ocluf's projects Team on Vercel.

A member of the Team first needs to authorize it.

ocluf commented 4 months ago

Hey @Creative-Difficulty,

Can you explain what you are trying to fix here? I have two specific migrate commands in the boilerplate. One without the -prod flag for local development and one with --prod for when you want to update the production database you have configured with turso.

This seems to remove that --prod flag

Creative-Difficulty commented 4 months ago

Hey @Creative-Difficulty,

Can you explain what you are trying to fix here? I have two specific migrate commands in the boilerplate. One without the -prod flag for local development and one with --prod for when you want to update the production database you have configured with turso.

This seems to remove that --prod flag

Yes, thats by design: As per your logic, in development file:local.db is hardcoded as the database url. If the project is being ran in development, the TURSO_DATABASE_URL would have to be set to file:mylocaltest.db (or any other name instead of mylocaltest) anyways for the database (lib/server/database/db.ts) to work with the local file. For this reason, the --prod flag is irrelevant and adds unnecessary complexity imo. I have replaced this with just reading the environment variable as it is also closer to the actual implementation (the one for the app, not the one for migrations) in lib/server/database/db.ts. And we want to run the database itself and the migrations with the same config, right?

Creative-Difficulty commented 4 months ago

@ocluf

ocluf commented 4 months ago

With this code change whenever I want to run a migration in production I have to change the TURSO_DATABASE_URL to the production url and afterwards make really sure I don't forget to set env variable back to file:local.db otherwise I accidentally mess up production when developing a new feature.

I rather have the explicit --prod flag so that I only migrate production when I really mean to so I'm going to reject this change.

Creative-Difficulty commented 4 months ago

that makes no sense, as the env variable also of course impacts the actual app so you have to change it anyways. @ocluf pls reopen

Creative-Difficulty commented 4 months ago

@ocluf

ocluf commented 4 months ago

This migration script is separate from running the app it is only used to migrate the database it doesn't run in production. And you want to make very sure you don't mistakenly run it with the wrong environment variable