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

use explicit full path to form more informative error messages #237

Merged
merged 2 commits into from
Jan 29, 2018

Conversation

maxbeatty
Copy link
Contributor

I've noticed quite a few issues and comments where it's hard to debug when a .env file isn't found. By using process.cwd explicitly, the full path will be included in the error message to aid in debugging. Hopefully this is an improvement from having to work backwards from a custom path to figure out what the original error message meant.

  • start testing Node v9 and drop v7 since it's no longer maintained
  • use process.cwd to declare a full path to read from
  • remove references to iojs
  • introduce some es6 syntax ref Feature: ES2015 #236

@maxbeatty maxbeatty self-assigned this Nov 2, 2017
@maxbeatty maxbeatty requested review from motdotla and jcblw November 2, 2017 21:53
@maxbeatty
Copy link
Contributor Author

I might suggest this be released as a MAJOR version since there's a slim possibility that some assumptions could break by changing the default path that is being read.

@coveralls
Copy link

coveralls commented Nov 2, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling b619d4d on better-err into 46ec97d on master.

@maxbeatty maxbeatty mentioned this pull request Nov 2, 2017
@coveralls
Copy link

coveralls commented Nov 9, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 93df1db on better-err into 46ec97d on master.

@maxbeatty
Copy link
Contributor Author

@motdotla will you configure AppVeyor for the repository to enable Windows testing? I think you need to do it as the repo owner. This will unblock #241 and a new release (https://github.com/motdotla/dotenv/projects/1)

Copy link

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

this looks great! I was confused by this documentation too, since no error was thrown for non-existing files.

@maxbeatty
Copy link
Contributor Author

I am going to merge this and release v5. #260 has been created to track adding AppVeyor

@maxbeatty maxbeatty merged commit 95aa929 into master Jan 29, 2018
@maxbeatty maxbeatty deleted the better-err branch January 29, 2018 22:50
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.

4 participants