Windows implementation of feature path_try_exists#85060
Windows implementation of feature path_try_exists#85060bors merged 2 commits intorust-lang:masterfrom
path_try_exists#85060Conversation
|
r? @yaahc (rust-highfive has picked a reviewer for you, use r? to override) |
67af57e to
1ddbb93
Compare
|
Hey @ChrisDenton, thank you for the PR and I'm sorry for the slow reply. I've taken a look, and admittedly I'm no Windows expert, but everything here looks good to me. I really appreciate the comments btw, they made reviewing this PR much easier. For a real Windows expert you could try asking on Zulip (in #t-libs or #t-compiler/windows) first. If that doesn't work, we can ping a few Windows experts on GitHub here. |
|
@yaahc No worries, I'm not in a rush! I've asked in the Windows group if someone wouldn't mind taking a look. I think there are a few changes I'd like to make but I'd prefer to wait for a second opinion first. |
1ddbb93 to
5d7c75a
Compare
|
A Microsoft engineer expressed the concern that using I think this is now ready for review. |
|
Looks good, thank you. @bors r+ |
|
📌 Commit 5d7c75a has been approved by |
Windows implementation of feature `path_try_exists` Draft of a Windows implementation of `try_exists` (rust-lang#83186). The first commit reorganizes the code so I would be interested to get some feedback on if this is a good idea or not. It moves the `Path::try_exists` function to `fs::exists`. leaving the former as a wrapper for the latter. This makes it easier to provide platform specific implementations and matches the `fs::metadata` function. The other commit implements a Windows specific variant of `exists`. I'm still figuring out my approach so this is very much a first draft. Eventually this will need some more eyes from knowledgable Windows people.
|
Failed in rollup: |
…forms can specialize it Windows implementation of `fs::try_exists`
|
Oh, sorry. I made a silly mistake there. I didn't catch it locally because I wasn't testing a target with an unsupported |
5d7c75a to
86dbc06
Compare
|
@bors r+ |
|
📌 Commit 86dbc06 has been approved by |
|
☀️ Test successful - checks-actions |
|
@ChrisDenton Hi. I'm doing some work on I think this was just a slip on your part, so I adjust that in my branch. If that was deliberate then please come over to #79965 and comment on e2bd0d0 Thanks :-). |
|
@ijackson That was indeed a slip on my part and I'm glad you caught it! |
No problem. They're the same type underneath anyway. Thanks. |
Draft of a Windows implementation of
try_exists(#83186).The first commit reorganizes the code so I would be interested to get some feedback on if this is a good idea or not. It moves the
Path::try_existsfunction tofs::exists. leaving the former as a wrapper for the latter. This makes it easier to provide platform specific implementations and matches thefs::metadatafunction.The other commit implements a Windows specific variant of
exists. I'm still figuring out my approach so this is very much a first draft. Eventually this will need some more eyes from knowledgable Windows people.