fix(contributing): minor corrections #201
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
benjie
commented
Oct 31, 2016
| scripts/dev | ||
| createdb postgraphql | ||
| scripts/run-kitchen-sink-sql postgraphql | ||
| scripts/dev -c pg://localhost/postgraphql |
There was a problem hiding this comment.
I felt uncomfortable installing schemas straight into the users default database; I can edit these changes back out if you prefer.
There was a problem hiding this comment.
How about we add a note instead that this is a valid alternative? Something like:
This will put the schemas into your default database. If you would prefer to put the schemas in another database pass a connection argument like so:
createdb postgraphql
scripts/run-kitchen-sink-sql postgres://localhost:5432/postgraphql
scripts/dev -c postgres://localhost:5432/postgraphql
There was a problem hiding this comment.
Ahhh! I was wondering why psql wasn't accepting the URL as an argument, didn't realise it needed postgres:// rather than pg://
benjie
commented
Oct 31, 2016
| @@ -40,7 +41,7 @@ scripts/test --watch | |||
| Now, only the tests in the files you have changed will be run. There are some slow tests in the PostGraphQL suite so hopefully this should make your development time faster. Once you are in watch mode, Jest will present you with some options you can use to better configure your testing experience. | |||
|
|
|||
| ### Snapshots | |||
| PostGraphQL makes use of the Jest snapshot feature. Even when you change small things in PostGraphQL the snapshot tests are likely to fail. This is OK, the snapshot tests are expected to fail. To make the snapshot tests pass again, run `scripts/test --watch` and then press `u` once the initial tests have run. Or run `scripts/test --updateSnapshot`. This will rerun the tests and changing the snapshot files in the repository. Commit the changes to the snapshots and the changed snapshots will be reviewed along with the rest of your changes in the PR review process. | |||
| PostGraphQL makes use of the Jest snapshot feature. Even when you change small things in PostGraphQL the snapshot tests are likely to fail. This is OK, the snapshot tests are expected to fail. To make the snapshot tests pass again, run `scripts/test --watch` and then press `u` once the initial tests have run. Or run `scripts/test --updateSnapshot`. This will rerun the tests and change the snapshot files in the repository. Commit the changes to the snapshots and the changed snapshots will be reviewed along with the rest of your changes in the PR review process. | |||
There was a problem hiding this comment.
Not sure why GitHub hasn't highlit it, but I changed the word "changing" to "change"
benjie
force-pushed
the
fix/contributing
branch
from
4289958
to
14266fe
Compare
|
@calebmer Changes made 👍 |
calebmer
approved these changes
Nov 1, 2016
|
Awesome looks good 👍 |
Belline
pushed a commit
to Belline/postgraphql
that referenced
this pull request
Dec 18, 2017
benjie
added a commit
that referenced
this pull request
Jan 27, 2020
* First test * Add directives * Skipping fields/fragments based on directives
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@calebmer I made these tweaks following your early merge of #197 (thanks for doing that!)
By the way, the
Types.moneytests are still failing for me; still haven't had a chance to dig in and figure out why. All the other tests are running well now though, thanks to the increased timeouts.