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

fix: combine aliases on windows base dirs (ie: X:\) (fixes: #577) #1080

merged 1 commit into from Jan 17, 2021


Copy link

@vaab vaab commented Dec 14, 2020

Illustrating a valid fix for my issue, that is still happening ;-)

I had a (very quick) look at running the tests, but unfortunately I had a lot of tests failing on the pristine source prior to any change. So I figured I missed a lot of dependencies or information to get this running properly. I'm afraid that I couldn't check that I didn't break anything properly before submitting that code change.

If you think this code change is definitively worth it, I'll take the time to add tests to your convenience if required.

Copy link

nedbat commented Dec 19, 2020

@vaab Thanks for sticking with this! I took a quick look at the change, and it seems straightforward enough. The tests are passing here on GitHub, so it seems good. If you could add a test of the specific problem, that would be great!


Signed-off-by: Valentin Lab <>
Copy link

Codecov Report

Merging #1080 (8f4e695) into master (37285cb) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1080      +/-   ##
- Coverage   91.30%   91.24%   -0.06%     
  Files          90       90              
  Lines       13358    13358              
  Branches     1493     1493              
- Hits        12196    12188       -8     
- Misses        951      959       +8     
  Partials      211      211              
Impacted Files Coverage Δ
coverage/ 84.11% <100.00%> (ø)
tests/ 90.82% <0.00%> (-7.34%) ⬇️

Copy link
Contributor Author

vaab commented Dec 26, 2020

Added the test as required, kept it quite simple. Thanks for your feedback. Should I worry about the coverage drop ? It seems to happen in a test file, which was seemingly untouched by my change.

Copy link

nedbat commented Jan 17, 2021

@vaab no worries about the codecov drop. Ironically, I can't make heads or tails of those reports, and have turned them off.


@nedbat nedbat merged commit dc0e806 into nedbat:master Jan 17, 2021
2 checks passed
nedbat added a commit that referenced this pull request Jan 17, 2021
Copy link

nedbat commented Jan 26, 2021

This is now released as part of coverage 5.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

None yet

3 participants