Skip to content
This repository has been archived by the owner on Sep 6, 2023. It is now read-only.

3/set up and scaffold (second attempt) #10

Merged
merged 29 commits into from
Jan 30, 2017
Merged

Conversation

AdamVig
Copy link
Collaborator

@AdamVig AdamVig commented Jan 29, 2017

This is the one to actually review.

Closes #10.

Copied from emmanuelroussel/stockpile-app.
Copied from Cinema Leagues and lightly edited.
The build is failing because the environment variables file was not
found on the build server. Since the actual variables are set in the
Travis CI environment and not a file, it should fix the problem to copy
the example environment variables file and allow empty values.
This allows remote access to the database instead of assuming that it
will always be available on localhost.
This should have been added when the same option was added in
`index.js`.
This should have been added when this environment variable was added in
the database service.
When the test fails, it does not clean up after itself, so the test must
drop its table (if it exists) before trying to create a new one.
The command added to the Travis CI configuration depends on existing
coverage output generated by `nyc`. Adding `nyc` to the test script
creates that output.
Since tests are written in ES2015+, they are not guaranteed to work with
all Code Climate engines.
@AdamVig AdamVig self-assigned this Jan 29, 2017
@ghost
Copy link

ghost commented Jan 29, 2017

Your pull request doesn't follow our guidelines. Please fix the following:

  • Pull request description must include a screenshot (?)
  • Pull request description must include verification steps (?)

Click here for details.

Thank you! 🙏

This comment was made by GitMagic – Magically enforcing your contribution guidelines.

@AdamVig AdamVig mentioned this pull request Jan 29, 2017
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling deb3ee7 on 3/set-up-and-scaffold into ** on master**.

Copy link
Collaborator

@emroussel emroussel left a comment

Choose a reason for hiding this comment

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

Are you planning on testing the other db operations in db.js later?

Also, you should probably close the issue this PR refers to in the body.

@AdamVig
Copy link
Collaborator Author

AdamVig commented Jan 30, 2017

@emmanuelroussel Good call. I added the issue to the body of the PR. And yes, see #5, I am planning to add tests for the database service in there (trying to keep this PR focused on environment setup).

@AdamVig AdamVig merged commit 77fc434 into master Jan 30, 2017
@AdamVig AdamVig deleted the 3/set-up-and-scaffold branch January 30, 2017 03:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants