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

Adds a note about the two commands you will use with your schema to the top of the schema file #8589

Merged
merged 4 commits into from
Dec 27, 2023

Conversation

orta
Copy link
Contributor

@orta orta commented Jun 12, 2023

It was years ago when I noted that I should send this to the template, I've never had to look at docs after adding these - they're a bit terse, open to changing the wording.

@jtoar jtoar added the release:chore This PR is a chore (means nothing for users) label Jun 12, 2023
@jtoar
Copy link
Contributor

jtoar commented Jun 12, 2023

@orta I'm with you that yarn rw prisma db push isn't advertised enough and is often a better workflow. I think my only concern here is that we don't mention it at all in the tutorial so I'm not sure if it'd create confusion for new users or not. But we add these kind of comments to the template files all the time, and they're meant for intermediate or advanced usage—when the time comes. @cannikin any thoughts?

@cannikin
Copy link
Member

Hmm, I've never used db push before...my flow is to edit the schema file and then migrate dev. I found this "when to use each" section on Prisma's site:

image

So if you prototype stuff and get it where you want with push do you do a single migrate dev at the end, and then all of the changes since the last migrate are turned into a single migration file? I found this doc which makes it sound like that migrate will often result in having to reset the database from scratch, which succckkksssss.

I don't know that the tutorial needs to update, doing it the migrate way works as is and could be considered "safer" since it shouldn't result in a reset. Do we have any other docs that talk about database workflow? It would be good to include this there for sure...

@orta
Copy link
Contributor Author

orta commented Jun 13, 2023

I've generally only used db push if I know that the schema probably isn't right but I want to explore the space (maybe 1 times for every 20 db migrate?) - I re-use staging data locally and so data loss from the exploration isn't a thing.

Could just remove it, it is an extra choice and then no-one has to try document the nuance of it?

@thedavidprice
Copy link
Contributor

We can do better across the project by referencing the docs for the specific tools+libraries in use. Prisma teaches both push and migrate in their docs, so we should hand off to them. And, frankly, we should like to the docs about schema.prisma:

tl;dr:

  • I really like the idea of updating the comments in this file
  • My 👍 is to direct people to Prisma docs, 'cause "teach a (wo)man to fish..."

@Tobbe
Copy link
Member

Tobbe commented Dec 27, 2023

The problem with including (too many) links is they go stale
(Case in point: prisma.io/docs/guides/migrate/developing-with-prisma-migrate no longer exists)

How about something like this?

// Don't forget to tell Prisma about your edits to this file using
// `yarn rw prisma migrate dev` or `yarn rw prisma db push`.
// `migrage` is safer, `push` can feel like less friction.
// Read more about both here:
// https://www.prisma.io/docs/orm/prisma-migrate

Copy link
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Paired with @Tobbe, looks good!

@jtoar jtoar added this to the next-release-patch milestone Dec 27, 2023
@jtoar jtoar merged commit 39cc3aa into redwoodjs:main Dec 27, 2023
6 checks passed
dac09 added a commit to dac09/redwood that referenced this pull request Dec 28, 2023
…p-prebuild

* 'main' of github.com:redwoodjs/redwood: (1608 commits)
  Docker: Update to work with corepack and yarn v4 (redwoodjs#9764)
  [RFC]: useRoutePaths (redwoodjs#9755)
  Adds a note about the two commands you will use with your schema to the top of the schema file (redwoodjs#8589)
  docs: Supertokens.md: Fix typo (redwoodjs#9765)
  Fix supertokens docs & integration issues (redwoodjs#9757)
  fix(apollo): Enhance error differently for Suspense Cells (redwoodjs#9640)
  SSR smoke-test: Use <Metadata /> (redwoodjs#9763)
  chore(deps): update dependency @types/qs to v6.9.11 (redwoodjs#9761)
  chore(ci): Better error handling in detectChanges.mjs (redwoodjs#9762)
  fix(path-alias): Fix aliasing of paths using ts/jsconfig (redwoodjs#9574)
  chore(deps): update dependency @types/yargs to v17.0.32 (redwoodjs#9759)
  Make it easier to find useMatch docs (redwoodjs#9756)
  chore(unit tests): Use side-effect import to fix TS errors (redwoodjs#9754)
  fix(context): Refactor context (redwoodjs#9371)
  docs: Replaced deprecated <Set private> with PrivateSet within router.md (redwoodjs#9749)
  add TS support for storybook preview tsx config extension (redwoodjs#9309)
  fix(studio): Fix windows path issues (redwoodjs#9752)
  chore(tasks): Add comparison view to nmHoisting visualisation (redwoodjs#9751)
  chore(cli): make fs modules used in the CLI consistent (redwoodjs#9746)
  ...
Tobbe pushed a commit that referenced this pull request Jan 1, 2024
…he top of the schema file (#8589)

It was years ago when I noted that I should send this to the template,
I've never had to look at docs after adding these - they're a bit terse,
open to changing the wording.
@Tobbe Tobbe modified the milestones: next-release-patch, v6.6.1 Jan 1, 2024
Tobbe pushed a commit that referenced this pull request Jan 1, 2024
…he top of the schema file (#8589)

It was years ago when I noted that I should send this to the template,
I've never had to look at docs after adding these - they're a bit terse,
open to changing the wording.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:chore This PR is a chore (means nothing for users)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants