Repository navigation
Refactor copyFile() message fragment for ease of translation - #6483
Conversation
MichaelChirico
added
the
translation
|
Generated via commit 78ad6d8 Download link for the artifact containing the test results: ↓ atime-results.zip Time taken to finish the standard R installation steps: 11 minutes and 42 seconds Time taken to run |
|
This approach (pull out the message generation logic from OTOH, it's only called in two places, so it's probably not worth over-thinking how to best modularize here. |
| // In future, we may discover a way to mmap fileSize+1 on all OS when fileSize%4096==0, reliably. If and when, this clause can be updated with no code impact elsewhere. | ||
| copyFile(fileSize, msg, verbose); | ||
| double time_taken = copyFile(fileSize); | ||
| if (time_taken < 0) { |
There was a problem hiding this comment.
In your experience as a professional software developer, does it pay to account for very unlikely corner cases, such as the system clock being very coincidentally adjusted backwards right as copyFile is running? If you think it's too unlikely to care about, that's probably fine.
Maybe we could return NaN on error, but that's a weird error return code.
There was a problem hiding this comment.
I was vaguely musing if that's possible as I wrote this.
I think we can wait for a user report before fussing here
copyFile should almost never be called. the two cases are (a) one-column files with multiple trailing missing rows and (b) files without trailing newline, in both cases only if the file is an exact multiple of 4096 bytes.
for all that to be true AND the clock to run backwards, it feels like the user is just messing with us :)
There was a problem hiding this comment.
one easy way to make this even more impossible to trigger by mistake would be to only check copy file==-1.0 instead of just being negative. then the user would somehow have to have all of the above with an impossibly exact regression of the system time.
There was a problem hiding this comment.
If someone manages to trigger exactly that, well, it would be too late to buy lottery tickets.
2 participants
One part of #6482. The complication here is that we always show the before- and after- message if
verbose=True; otherwise, we take care to only flag this to the user if the required file copy is particularly time-consuming (>.5 seconds).It's also a tad complicated because there's a separation of logic to several places in the code...
That inspired refactoring
copyFile()to return the time taken -- now all the messaging logic surroundingcopyFile()is all done in one place --> easier to follow.