Skip to content

api returns: copy returns copied target file#259

Merged
borkdude merged 1 commit into
masterfrom
lread/api-review-step3b
May 11, 2026
Merged

api returns: copy returns copied target file#259
borkdude merged 1 commit into
masterfrom
lread/api-review-step3b

Conversation

@lread

@lread lread commented May 11, 2026

Copy link
Copy Markdown
Contributor

Step 3b for #197 (comment)

Docstring was slightly incorrect. We don't always return target-path, we return the copied target file. For example, (fs/copy "file" "dir") returns dir/file, not dir.

For consistency, moved "Returns..." in docstring above Options.

Renamed source-file arg to source; it can also be an input stream. (This was announced in changelog but never documented)

When source was input-stream we were returning the number of bytes copied, corrected to return the copied target file.

Tests for copy adjusted accordingly.

Please answer the following questions and leave the below in as part of your PR.

Step 3b for #197 (comment)

Docstring was slightly incorrect. We don't always return `target-path`,
we return the copied target file. For example, `(fs/copy "file" "dir")`
returns `dir/file`, not `dir`.

For consistency, moved "Returns..." in docstring above Options.

Renamed `source-file` arg to `source`; it can also be an input stream.
(This was announced in changelog but never documented)

When `source` was input-stream we were returning the number of bytes
copied, corrected to return the copied target file.

Tests for `copy` adjusted accordingly.
@borkdude borkdude merged commit 10ed59c into master May 11, 2026
30 checks passed
@borkdude borkdude deleted the lread/api-review-step3b branch May 11, 2026 18:45
@borkdude

Copy link
Copy Markdown
Contributor

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants