Skip to content

Comments

dist/cache: include paths in error contexts#1611

Merged
Xuanwo merged 1 commit intomozilla:mainfrom
mathstuf:dist-dirname
Feb 18, 2023
Merged

dist/cache: include paths in error contexts#1611
Xuanwo merged 1 commit intomozilla:mainfrom
mathstuf:dist-dirname

Conversation

@mathstuf
Copy link
Contributor

@mathstuf mathstuf commented Feb 18, 2023

Fixes: #885

In the interest of helping 0.4.0 get finalized :) .

@codecov-commenter
Copy link

codecov-commenter commented Feb 18, 2023 •
edited
Loading

Codecov Report

Base: 30.77% // Head: 30.84% // Increases project coverage by +0.06% 🎉

Coverage data is based on head (b02e044) compared to base (59e906d).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1611      +/-   ##
==========================================
+ Coverage   30.77%   30.84%   +0.06%     
==========================================
  Files          49       49              
  Lines       16611    16618       +7     
  Branches     8044     8049       +5     
==========================================
+ Hits         5112     5125      +13     
+ Misses       6217     6213       -4     
+ Partials     5282     5280       -2
Impacted Files Coverage Δ
src/dist/cache.rs 20.37% <0.00%> (-0.55%) ⬇️
src/test/tests.rs 33.33% <0.00%> (-0.69%) ⬇️
src/compiler/compiler.rs 36.03% <0.00%> (-0.08%) ⬇️
src/compiler/rust.rs 33.81% <0.00%> (-0.07%) ⬇️
src/lib.rs 11.37% <0.00%> (ø)
src/compiler/args.rs 62.43% <0.00%> (+0.17%) ⬆️
src/server.rs 31.08% <0.00%> (+0.20%) ⬆️
src/compiler/c.rs 38.16% <0.00%> (+0.22%) ⬆️
src/compiler/gcc.rs 54.75% <0.00%> (+0.30%) ⬆️
src/util.rs 37.50% <0.00%> (+1.04%) ⬆️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Xuanwo
Xuanwo approved these changes Feb 18, 2023
View reviewed changes
Copy link
Collaborator

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Thanks for contribution, LGTM!

@Xuanwo Xuanwo merged commit 4b53aca into mozilla:main Feb 18, 2023
@sylvestre
Copy link
Collaborator

sylvestre commented Feb 18, 2023

Would it be possible to have tests to cover these errors?

@mathstuf mathstuf deleted the dist-dirname branch February 18, 2023 16:16
@mathstuf
Copy link
Contributor Author

mathstuf commented Feb 18, 2023

Maybe? What fs abstractions do we have available? FUSE is a possibility, but feels like a lot of code to test error paths. Also doesn't help with Windows at all.

@drahnr
Copy link
Collaborator

drahnr commented Feb 18, 2023

Is there a particular reason to not use fs-err instead?

@mathstuf
Copy link
Contributor Author

mathstuf commented Feb 19, 2023

That seemed like a wider-effecting change than the issue was asking for. And given that 0.4.0 is now "one" issue away by milestoned issues…I'd like to see 0.4.0 come out first personally.

@drahnr drahnr mentioned this pull request Feb 21, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Xuanwo Xuanwo Xuanwo approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

dist-client does not print directory path when cache dir creation fails.

5 participants

@mathstuf @codecov-commenter @sylvestre @drahnr @Xuanwo