Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make code more ergonomic #59

Merged
merged 1 commit into from Mar 2, 2017
Merged

Make code more ergonomic #59

merged 1 commit into from Mar 2, 2017

Conversation

matt2xu
Copy link
Contributor

@matt2xu matt2xu commented Jan 17, 2017

Solves #56

  • remove unnecessary allocations (calls to format! and String::from)
  • make write_str accepts S: AsRef so we can pass a String without having to borrow it manually
  • make list_command accept Cow<'static, str> (better than AsRef because we can have both a &str or a String at calling site)
  • retr is generic over its result type (backward-compatible change)
  • rewrite retr in a more ergonomic way
  • rewrite simple_retr to use retr, remove simple_retr_

@mattnenterprise
Copy link
Owner

Currently we don't have tests. Have you tested the retr changes manually against an FTP server ? Just to make sure. Thats the only change I can see that would break things.

@matt2xu matt2xu mentioned this pull request Jan 20, 2017
Merged
Make code more ergonomic
b1f2743 
  * remove unnecessary allocations (calls to format! and String::from)
  * make write_str accepts S: AsRef<str> so we can pass a String without having to borrow it manually
  * make list_command accept Cow<'static, str> (better than AsRef because we can have both a &str or a String at calling site)
  * retr is generic over its result type (backward-compatible change)
  * rewrite retr in a more ergonomic way
  * rewrite simple_retr to use retr, remove simple_retr_

Possible backward-incompatible improvements, requiring major version changes:

  - make simple_retr return a Vec rather than a Cursor
  - change the signature of retr from Fn(&mut Read) to FnMut(R) where R: Read?
@matt2xu
Copy link
Contributor Author

matt2xu commented Jan 24, 2017

Rebased on top of 20422f0

@tomverran tomverran mentioned this pull request Feb 19, 2017
Closed
@mattnenterprise
Copy link
Owner

Thanks for the cleanup!

@mattnenterprise mattnenterprise merged commit 3de12e1 into mattnenterprise:master Mar 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@matt2xu @mattnenterprise