Don't rename unless file is in same dir#603
Don't rename unless file is in same dir#603rdimitrov merged 4 commits intotheupdateframework:masterfrom
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #603 +/- ##
==========================================
+ Coverage 70.18% 70.54% +0.36%
==========================================
Files 10 10
Lines 2123 2122 -1
==========================================
+ Hits 1490 1497 +7
+ Misses 517 504 -13
- Partials 116 121 +5 ☔ View full report in Codecov by Sentry. |
kipz
left a comment
kipz
left a comment
There was a problem hiding this comment.
Hard to test this one on GHA. Works for me at least.
rdimitrov
left a comment
rdimitrov
left a comment
There was a problem hiding this comment.
Thanks for addressing this! 🚀
@kipz - Thank you for testing this! 💯 My assumption is the examples that we run now both on windows and linux should be enough to catch potential errors of this, i.e. a temp file failed to be renamed. On that note, I'll go ahead and enable the macos environment too 👍 |
|
Here's the PR enabling the runner environments - #604. My assumption was we enabled the windows one for all, but it was just for the unit tests. Now it's going to run the examples too 👍 @jonnystoten - Now that I enabled the runners in the PR I see some of the examples for windows do fail. I don't have a windows machine in hand, do you think you can take a look while you're at this topic? I can give it a go using the CI later today or tomorrow too. |
rdimitrov
dismissed
their stale review
Let's wait until the CI for windows is green.
This was originally done for windows only, but renaming can cause issues regardless of OS if the target dir is on another device. Signed-off-by: Jonny Stoten <[email protected]>
rdimitrov
force-pushed
the
fix-cross-device-rename
branch
from
6d2722f to
aea0415
Compare
rdimitrov
left a comment
rdimitrov
left a comment
There was a problem hiding this comment.
When we enabled the windows runner it manifested some errors which is not surprising since we haven't tested go-tuf-metadata at all on Windows. I've opened an issue about it if someone is interested in addressing this - #605.
I'm okay to accept this PR since it doesn't seem related to the issue I mention 👍
|
Hey thanks @rdimitrov ! I actually don't have a windows machine either but I could have a look using the CI to test, the failure I can see looks pretty straightforward. |
4 participants
This was originally done for windows only, but renaming can cause issues regardless of OS if the target dir is on another device.