fix: For Windows, close temp file before removing #22492
Conversation
| @@ -218,7 +218,7 @@ func (cmd *Command) parseFlags(args []string) (err error) { | |||
| return err | |||
| } | |||
|
|
|||
| func (cmd *Command) backupShard(db, rp, sid string) error { | |||
| func (cmd *Command) backupShard(db, rp, sid string) (err error) { | |||
There was a problem hiding this comment.
could we call this returnErr or rErr or something? That seems cleaner than renaming the other errors later and avoids the risk of potential shadowing in the future.
There was a problem hiding this comment.
That was my first fix (see initial commit), calling it simply e. But I decided that preserving the convention of the return be called err was cleaner. More code changes now, but more understandable code in a year.
There was a problem hiding this comment.
I find returnErr = err much more understandable (now and in a year) because I can easily see I'm intending to change the return value and I can easily see that it is never shadowed (since I only assign to it in defer statements). I don't find that a naming convention for named error returns has much value, and I'd prefer if our convention was to specifically not name them err - i.e. func(myval Value) (err error) should be considered harmful, func(myval Value) error or func(myval Value) (returnErr error) or func(myval Value) (ret1 ReturnType1, _ error) is better in my opinion.
That said, not worth blocking the PR on.
Close temp files before removing them, because
Windows does not permit open files to be removed.
Closes #21470