search

BenChung / KRPC.jl

A kRPC client for Julia
MIT License
3 stars 4 forks source link

Various fixes for KRPC.jl #13

Closed Rhahi closed 1 year ago

Rhahi commented 1 year ago

Hello,

I am not sure if you like to receive PR with one PR doing one thing each, or receive fixes all at once, so I collected my accumulated fixes in one PR. If you prefer the other way around, we can close this and try again.

Here are the list of changes:

Minor changes

Fixes

BenChung commented 1 year ago

This format is fine - sorry, I'm not in a place where I can really review/help much right now.

The one thing I'm concerned about is the Tuple_ to Tuple change. That was done originally to dodge Base.Tuple; does it still work now?

On Sat, May 20, 2023, 6:22 AM Rhahi @.***> wrote:

Hello,

I am not sure if you like to receive PR with one PR doing one thing each, or receive fixes all at once, so I collected my accumulated fixes in one PR. If you prefer the other way around, we can close this and try again.

Here are the list of changes: Minor changes

-

2cc9c2e https://github.com/BenChung/KRPC.jl/commit/2cc9c2ee36df3c13f75f37df87eae9939f97d90f (HEAD -> develop, fork/develop) doc: add helpful inline comments for close(Listener) this contains simple commentaries to help myself. Maybe you don't want it? We can drop this commit.

4ddc0df https://github.com/BenChung/KRPC.jl/commit/4ddc0df0fcd3879740cd76ab01a4fc0ae74d4678 refactor: downgrade error() in close(Listener) to warning I thought throwing an error here is abit overkill, although having a request to closed stream might indicate that something else is wrong and having come "de-sync" issues. Let me know if you think this is a bad idea.

17fd742 https://github.com/BenChung/KRPC.jl/commit/17fd742134ef9afe58d0c2fa3fc97ef11d751bfe fix: remove unused PipeBuffer Probably a leftover from past code?

Fixes

-

a13a707 https://github.com/BenChung/KRPC.jl/commit/a13a707fdf77065941872bdd5f346f70d24e83ad fix: remove undescore in Tuple_ from types I think this is the one you told me in KRPC discord..

1942084 https://github.com/BenChung/KRPC.jl/commit/19420843b00bdc008ff90d8b546e5252b267cc0e fix: use one_to_many to checking unbound stream removal This is a fix for a regression I introduced. I was not properly handling closing of streams.

47344ab https://github.com/BenChung/KRPC.jl/commit/47344abf1d87573e6c53a8eabd48a8ecda7ffcf1 fix: close the Julia channel when closing listener This signals the consumer that the channel is closed and there is no point in waiting anymore.

ff8e994 https://github.com/BenChung/KRPC.jl/commit/ff8e994143ce5a9c9ffabaf78b5b2430b8edb53d fix: use try-finally to release semaphore I pointed this out after you merged my previous fix for raw.jl, and it's finally getting patched.

You can view, comment on, or merge this pull request online at:

https://github.com/BenChung/KRPC.jl/pull/13 Commit Summary

File Changes

(3 files https://github.com/BenChung/KRPC.jl/pull/13/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/BenChung/KRPC.jl/pull/13, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACADVV3FVB4B22VO4JFHBDXHDAQ7ANCNFSM6AAAAAAYIXUYNE . You are receiving this because you are subscribed to this thread.Message ID: @.***>

Rhahi commented 1 year ago

I just looked back on Discord chat log. You did not mention this line before; it's something I changed on my own. I probably changed it because something didn't work, I will change it back to Tuple_ and see how it goes again.