Fix preserving timestamp when the move command is executing

jprichardson / node-fs-extra

Node.js: extra methods for the fs object like copy(), remove(), mkdirs()

MIT License

9.43k stars 775 forks source link

Fix preserving timestamp when the move command is executing #994

Closed KostiantynPopovych closed 1 year ago

KostiantynPopovych commented 1 year ago

This PR resolves issue: #992.

These test files are pretty similar, I just took the copy method test files as a blueprint, and it seems we can make some refactoring here and there. I'm open to any suggestions!

RyanZim commented 1 year ago

https://github.com/jprichardson/node-fs-extra/issues/992#issuecomment-1414186359

KostiantynPopovych commented 1 year ago

Hey @RyanZim, I updated the implementation to match the logic that was discussed in thread #992. I refactored a bit methods move & moveSync to match the same style.

RyanZim commented 1 year ago

It's not clear to me why the large refactor is needed; theoretically you only need to set preserveTimestamps in moveAcrossDevice, similarly to how errorOnExist is currently set. What am I missing?

KostiantynPopovych commented 1 year ago

@RyanZim Reverted original code, that refactoring might be overengineering for now 🤔

KostiantynPopovych commented 1 year ago

Thank you for reviewing it!