Skip to content

Make Controller and Listener impl Send - #46

Merged
AldaronLau merged 1 commit into
mainfrom
send
Feb 28, 2023
Merged

Make Controller and Listener impl Send#46
AldaronLau merged 1 commit into
mainfrom
send

Conversation

@AldaronLau

Copy link
Copy Markdown
Member

This makes stick easier to work with when using tokio.

@hkoosha

hkoosha commented Feb 27, 2023

Copy link
Copy Markdown

Hi

Is there a plan to merge this? and if not, does it make sense if I maintain and publish a fork on the crates.io, such as sync-threaded (or maybe with a better name), that solely has this PR applied?

I'm planning to publish a crate, it uses tokio and currently a local fork of stick with Arc applied. The local fork makes it unsuitable for crates.io.

If you don't like this fundamental change applied to the original stick, maybe using Cargo features an option too? I can make a PR for that too.

@AldaronLau

Copy link
Copy Markdown
Member Author

@hkoosha Sorry, I honestly don't remember why I left this open (I probably just forgot about it). I can totally merge it. I'll try to get a new version with this patch out soon (at least sometime this week) for crates.io.

Also slightly off topic, but

If you're interested, this project has been a little bit difficult for me to maintain on my own (with the scope of this project and the fact I maintain ~70 crates), so I will be looking to add more maintainers soon. Since you mentioned an interest in maintaining a fork, and have a crate that depends on stick, I think you could be a good candidate for an additional maintainer. Let me know if you would be interested.

I think maybe the reason I didn't merge it, is to avoid too much divergence from a major rewrite of a lot of the code I started about a year ago but haven't been working on consistently. I think it's small enough it should be fine, though. The main blocker for merging my rewrite is that it currently breaks functionality on Windows. I'm considering merging it as-is just to get the momentum going again, and then revisit after a new crates.io release to fix the Windows breakage.

The rewrite is in this PR: #50

@AldaronLau
AldaronLau merged commit c09e07d into main Feb 28, 2023
@AldaronLau
AldaronLau deleted the send branch February 28, 2023 01:06
@hkoosha

hkoosha commented Feb 28, 2023

Copy link
Copy Markdown

I would be super interested in helping and becoming a maintainer!

I'll look into the Windows issue this week.

@AldaronLau

Copy link
Copy Markdown
Member Author

@hkoosha Thank you, I think having an additional maintainer will be really great for this project! I'll look into giving you permission to merge PRs and push releases out to crates.io once I transfer this repository to the https://github.com/ardaku organization (hopefully this week still, but might end up being next week).

And feel free to reach out with any questions you have, about the change or otherwise.

@AldaronLau

Copy link
Copy Markdown
Member Author

@hkoosha I released stick version 0.13.0 with the patch.

I also sent an invite to you to become a maintainer that should show up in an email. It should let you publish releases on crates.io, merge PRs, and create branches in this repository. Here's some of the next things on my list of maintenance tasks to do with stick, that you could pick up if you want to:

Let me know which tasks you are planning on doing (so we don't do the same work), and if you can request a code review from me before merging changes that would also be appreciated. I will also start requesting code reviews from you for changes I want to make. Some of the tasks may be very difficult and time-consuming, with #50 changing a lot of the code, but I think if we can chip away at it, we can start to accelerate development on stick again. Thanks again for volunteering to help maintain stick! Hopefully everything here is not too overwhelming, and let me know if there's anything you think I could do better.

@hkoosha

hkoosha commented Mar 5, 2023

Copy link
Copy Markdown

Many thanks for the invitation! Excited to be part of the project. (And thanks for the new release, makes life much easier now).

I would like to start looking into this if possible:

Figure out why CI on stable branch is broken (windows-gnu target weirdness)

I'm not a windows savy person so it might take me a few days more than usual to get it right.

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