search

SLMT / telnet-rs

A simple implementation of Telnet in Rust.
MIT License
44 stars 20 forks source link

Rust 2018 migration #18

Closed SaadiSave closed 2 years ago

SaadiSave commented 3 years ago

Is this crate planning to upgrade to Rust 2018? I can contribute a PR if needed.

SLMT commented 3 years ago

Sure! BTW, I am not sure what should be modified for Rust 2018, is there any reference for this?

SaadiSave commented 3 years ago

You can use cargo migrate after adding edition = 2018 to Cargo.toml. I have already finished migration to 2018, using a different method:

  1. Set the edition in Cargo.toml
  2. Follow all rust-analyzer and clippy suggestions in the Vscode.

I will be making a PR soon.

Btw, what's the copyright situation like? Do I add my name to the copyright or something? Please clarify that.

I still have to write some tests. I also have ideas for an async implementation, if you'd like to work on it with me.

SLMT commented 3 years ago

Ok. Sounds good!

I totally forgot that I use my name in the Copyright statement. Do you think if it would be better to replace my name with telnet.rs Contributors?

We can discuss the aynsc implementation after we merge the PR.

SaadiSave commented 3 years ago

Something like Copyright (c) 2017-2021 Yushan Lin and telnet-rs contributors and then maybe store the contributors list in Contributors.md?

Also, I observed that you have not renewed the copyright since 2017. Renewing it every year is recommended.

SLMT commented 3 years ago

Thanks for your advice. I will update the Copyright statement and add Contributors.md immediately.

SLMT commented 3 years ago

@SaadiSave Could you help me check if this PR looks good for you?

SaadiSave commented 3 years ago

Yes, looks good. Btw, I should be done documenting my changes by Friday.

SLMT commented 3 years ago

Great! I'm looking forward to seeing it.

SaadiSave commented 2 years ago

I've added some code for error handling, so that errors are not just strings anymore. I've also replaced single variant matches with if let. After a few more reviews, and the PR should land before Sunday.

SaadiSave commented 2 years ago

The PR has landed