dotenv-rs / dotenv

Library to help supply environment variables in testing and development
MIT License
566 stars 45 forks source link

Show symbol index that caused an error #7

Closed SomeoneToIgnore closed 5 years ago

SomeoneToIgnore commented 5 years ago

When the line parsed with an error is long, say DATABASE_URL="postgres://${POSTGRES_USER}:${POSTGRES_PASSWORD}@localhost:5432/$\{POSTGRES_DB}"

it's rather hard to understand which symbol had caused a parsing error.

This PR makes it a bit more clearer. It would be even more descriptive to actually highlight the wrong symbol instead with some symbol, say ^, but not sure if it's a good approach for the library crate.

codecov[bot] commented 5 years ago

Codecov Report

Merging #7 into master will increase coverage by 0.32%. The diff coverage is 88.88%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #7      +/-   ##
=======================================
+ Coverage   90.67%   91%   +0.32%     
=======================================
  Files          18    18              
  Lines         547   567      +20     
=======================================
+ Hits          496   516      +20     
  Misses         51    51
Impacted Files Coverage Δ
dotenv_codegen_implementation/src/lib.rs 0% <0%> (ø) :arrow_up:
dotenv/src/errors.rs 90.47% <80%> (ø) :arrow_up:
dotenv/src/parse.rs 97.41% <93.33%> (-0.59%) :arrow_down:
dotenv/tests/test-from-filename-iter.rs 90% <0%> (ø) :arrow_up:
dotenv/src/lib.rs 100% <0%> (ø) :arrow_up:
dotenv/tests/test-dotenv-iter.rs 90% <0%> (ø) :arrow_up:
dotenv/tests/test-from-path-iter.rs 91.66% <0%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 26b288a...872e6d8. Read the comment docs.

SomeoneToIgnore commented 5 years ago

I don't understand the codecov status, is it stating that the PR had increased the coverage but it's not enough still?

Looks rather unfair to me.

ZoeyR commented 5 years ago

It's just saying that of the code changed 88% was covered whereas the target is 90%. There is a reason I don't have codecov listed as a required status.