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

Update license, dependencies, and environmental variable loading #14

Merged

Conversation

jniles
Copy link
Collaborator

@jniles jniles commented Jan 11, 2016

This commit updates bhima's license to an SPDX valid license identifier (we are still GPL version 2). It also updates the outdated dependencies in the package.json. In particular, we no longer use the deprecated gulp-minify-css and instead use gulp-cssnano. See this page. Closes #15.

Breaking Change This PR depends on the dotenv npm package to manage environmental variables. You must have a .env.production file in the top level folder of the repository to run the application using npm run app. To run tests, you must have a .env.development file. Closes #4.

@jniles jniles changed the title CHORE(package): updated dependencies and license Update license, dependencies, and environmental variable loading Jan 12, 2016
@jniles
Copy link
Collaborator Author

jniles commented Jan 12, 2016

@sfount can you do a test of this branch before a merge to master?

This commit updates bhima's license to an SPDX valid license identifier
(we are still GPL version 2).  It also updates the outdated
dependencies in the package.json.
This commit removes the deprecated dependency on gulp-minify-css and
replaces it with gulp-cssnano in the build.  It also fixes a few css
rules discovered by the build process.
This commit migrates all the server code to use the dotenv npm module to
manage environmental variables.  For reference, a ".env.sample" file is
included and should be customized to ".env.production" for production
mode and ".env.development" for the development environment.  All
previous configuration files have been removed.

Additionally, much ancient commented code has been cleaned up from the
database library code.
@sfount
Copy link
Contributor

sfount commented Jan 19, 2016

@jniles - missed this message 👎, will have it in this morning.

@jniles
Copy link
Collaborator Author

jniles commented Jan 19, 2016

👍

@sfount
Copy link
Contributor

sfount commented Jan 19, 2016

Everything looks good; I think ensuring the default environment variables are configured would be a good next step for some build/ install script (for the future).

Before this can be pulled all references to server/config/environment/* will need to be removed. Deleting the bin folder reveals these dependencies.
I suggest:

  • Including the server-clean gulp build process in the the gulp build command (ensuring the bin file is removed for every test)
  • Updating all references to the old configuration files.

Cheers.

@jniles
Copy link
Collaborator Author

jniles commented Jan 19, 2016

For reference (@IMA-WorldHealth/local-contributors), .env.production and .env.development have now been uploaded to the [email protected] google drive account.

EDIT also, thanks @sfount for the review. Will get these changes in soon.

This commit fixes a danging config not updated to use process.env.  This
commit ensure the budget module properly looks up the environmental
variables from the `process` object.

Additionally, this commit ensures that the `/bin` directory is deleted
and recreated when running the gulp-build or `npm run app` commands.
@jniles jniles force-pushed the patches-update-dependencies branch from 5b9c11c to d354e01 Compare January 19, 2016 12:19
@jniles
Copy link
Collaborator Author

jniles commented Jan 19, 2016

@sfount, the fixes you requested have landed in this branch. Thanks!

sfount added a commit that referenced this pull request Jan 19, 2016
Update license, dependencies, and environmental variable loading
@sfount sfount merged commit 00d9c50 into Third-Culture-Software:master Jan 19, 2016
@jniles jniles deleted the patches-update-dependencies branch January 19, 2016 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants