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

ReadME documentation on migrations may be misleading #507

Closed
msdrigg opened this issue Jun 21, 2022 · 1 comment
Closed

ReadME documentation on migrations may be misleading #507

msdrigg opened this issue Jun 21, 2022 · 1 comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@msdrigg
Copy link

msdrigg commented Jun 21, 2022

Improve documentation

Link

https://github.com/supabase/gotrue/blob/c9f1ed5731b102a40bae113e972c0e7674a9195b/README.md?plain=1#L136

Describe the problem

I believe that migrations are run automatically whenever gotrue is started. I am basing this believe on the fact that the root command calls migrate before calling any other command. See here:
https://github.com/supabase/gotrue/blob/c9f1ed5731b102a40bae113e972c0e7674a9195b/cmd/root_cmd.go#L15

Describe the improvement

This section should specify that migrations are run automatically when gotrue is called and do not need to be run with gotrue migrate at the start.

Additional context

I am just getting into go, and I am reading over the source of gotrue to get a feel for the language. All statements in this issue may be totally false, because I could be totally misunderstanding the code I am reading. If this is the case, please show some patience in the replies.

Final Note

It might also be helpful to add a branch so that migrations are not run twice when gotrue migrate is called. Because currently, when gotrue migrate is called, migrations are run once here: https://github.com/supabase/gotrue/blob/c9f1ed5731b102a40bae113e972c0e7674a9195b/cmd/root_cmd.go#L15
and then run again here https://github.com/supabase/gotrue/blob/c9f1ed5731b102a40bae113e972c0e7674a9195b/cmd/migrate_cmd.go#L19

But I don't think that this double-run isn't really doing any harm so no need to special-case it if y'all think it's fine.

@msdrigg msdrigg added the documentation Improvements or additions to documentation label Jun 21, 2022
@J0
Copy link
Contributor

J0 commented Jun 21, 2022

Hey @msdrigg,

Thanks for the comprehensive description on the issue. The documentation does seem to be a little dated -- migrations are applied on startup and we now have a docker workflow for development. PRs are welcome!

With regards to the comment about the migrate command -- I think the main command does not run when the user chooses to run the sub command (e.g. only commands associated with ./gotrue migrate will be called and not both the main command( ./gotrue) and ./gotrue migrate

@J0 J0 added the good first issue Good for newcomers label Jun 28, 2022
@J0 J0 mentioned this issue Jun 29, 2022
@J0 J0 closed this as completed Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants