-
Notifications
You must be signed in to change notification settings - Fork 385
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
docs: update contributors guide #285
Conversation
Thanks @kangmingtay I had some mods, too which I can add to this branch or suggest here:
go install github.com/gobuffalo/pop/soda@latest
Few tips in:
and then the section about if one has Postgres already running:
///file: postgresd.sh
// file: database.yaml
// file: test.env
//file: migrate.sh
Also, there are a few install and helpful commands you shared with me during installation:
That
We set real GOTRUE_SMTP_HOST settings. If people do not have access to that, what do they use?
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some notes that I had been keeping in my Contribute and Readme that helped my install.
Le me know if you want me to work them into your PR and happy to.
A point on that @dthyresson, Make sure not to suggest brew as an install method as it doesn't work on M1 MacBooks/Macs and that would fix #280 |
@kangmingtay Should this PR include the guidelines for splitting work into PRs by feature or by individual bug fixes? Mentioned here: #269 (comment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kangmingtay for the improvement, i will try to play the install again to report better issue around install process
CONTRIBUTING.md
Outdated
* `go install github.com/gobuffalo/pop/soda@latest` | ||
* Clone this repo: `git clone https://github.com/supabase/gotrue` | ||
* `cd gotrue` | ||
* To start the gotrue postgresql container running locally: `./hack/postgresd.sh` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
./hack/postgresd.sh
didn't work when we tried there error in the script from dbname I think @HarryET you remember ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @riderx, sorry for the late reply, thanks for pointing this out, are you able to recall the error message you encountered? 🤔
CONTRIBUTING.md
Outdated
- Execute the binary `./gotrue` | ||
- gotrue runs any database migrations from `/migrations` on start | ||
- Start PostgreSQL inside a docker container running `./hack/postgresd.sh` | ||
- Run `make migrate_test` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same in migration we need to check the script work from 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you elaborate by what you mean by "work from 0" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a huge improvement, thanks both!
fb5ef9d
to
d9a6ede
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved.
🎉 This PR is included in version 2.2.14 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
What kind of change does this PR introduce?