Skip to content

Update Module with Tests and 100% Test Coverage#8

Open
nelsonic wants to merge 9 commits intodshaw:masterfrom
dwyl:master
Open

Update Module with Tests and 100% Test Coverage#8
nelsonic wants to merge 9 commits intodshaw:masterfrom
dwyl:master

Conversation

@nelsonic
Copy link
Collaborator

@nelsonic nelsonic commented Jul 17, 2015

Hi @dshaw,
As promised: #6 here's the Pull Request with the update to latest node.js (apologies for taking so long, we got "distracted" with our work... 😉 )

Build Status Test Coverage Code Climate Dependency Status devDependency Status

What's included in the PR?

  • migrated tests from Tap to Tape so that we could collect code coverage info with Istanbul.
  • added istanbul for code coverage (100%)
  • sends code coverage stats to codeclimate.com (which also tracks code quality)
  • incremented the version number to 1.0.0 to be Semver-compliant
  • removed one line of unused/untested code from lib/env.js
  • but, did not change any functionality in your module (so it should be 100% backward compatible)
  • added pre-commit hook to check coverage so future contributions do not reduce test covreage
  • added a couple more badges to the readme so that its clear to people that this module is maintained/loved! (the links are to our travis/codeclimate/dependencies we intend to keep the fork in our org for reference but the readme links can easily be changed with a find-and-replace if you like)

Please let us know if this is ok. otherwise let us know what you would prefer us to change.

Thanks!
N 🚀

@nelsonic nelsonic mentioned this pull request Jul 17, 2015
Open
nelsonic
nelsonic reviewed Jul 17, 2015
View reviewed changes
lib/env.js
Copy link
Collaborator Author

@nelsonic nelsonic Jul 17, 2015

Choose a reason for hiding this comment

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

We use fs.statSync because if the env.json file does not exist most apps probably wont work to the error (with full stack trace) is appropriate. what do you think?

@nelsonic
Copy link
Collaborator Author

nelsonic commented Jul 30, 2015

@dshaw please let us know if anything is missing to get this PR merged.
I promise I won't be offended if you tell me to rip out chunks of it because I've gone too far or something...
We just really want to use your env module in our app(s) but can't until this update goes in ...
thanks!

@JoseCage
Copy link

JoseCage commented Aug 15, 2017

Why this PR stills Open?? (not merged 🐛 🐛 )

@nelsonic nelsonic mentioned this pull request Jul 17, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nelsonic @JoseCage