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.

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

Add a debug implementation for Address #67

Merged
merged 16 commits into from
Jun 9, 2022
Merged

Add a debug implementation for Address #67

merged 16 commits into from
Jun 9, 2022

Conversation

Restioson
Copy link
Owner

@Restioson Restioson commented Apr 10, 2022

This does not address MessageChannel as I just haven't looked into it yet. I'll look into it before this gets merged, though. Closes #54.

  • Implement Debug on RefCounter and show reference counts
  • Implement for MessageChannel
  • Add test

@Restioson Restioson self-assigned this Apr 10, 2022
@Restioson Restioson added the enhancement New feature or request label Apr 10, 2022
@Restioson Restioson added this to the 0.6.0 milestone Apr 10, 2022
Copy link
Collaborator

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that is a good starting point. Implementing Debug on RefCounter could also be interesting to see the actual refcount.

@Restioson
Copy link
Owner Author

That makes sense

@Restioson
Copy link
Owner Author

Is this what you were thinking of when it comes to the reference count debugging @thomaseizinger ?

@thomaseizinger
Copy link
Collaborator

Yeah this looks good! Perhaps we can add a test that asserts the Debug implementation? This would make it even easier to review and we make sure it is not broken :)

src/address.rs Outdated Show resolved Hide resolved
@Restioson
Copy link
Owner Author

Sounds like a good plan

@Restioson
Copy link
Owner Author

I've added a test for this now. One remaining unresolved question is whether to report the weak count strictly accurately (i.e number of weak references, incl. the one kept inside Context), or to make it count the number of weak addresses (statically one less than the strict count).

@thomaseizinger
Copy link
Collaborator

I've added a test for this now. One remaining unresolved question is whether to report the weak count strictly accurately (i.e number of weak references, incl. the one kept inside Context), or to make it count the number of weak addresses (statically one less than the strict count).

I think reporting the actual number of addresses is good enough. I see very little foot-gun potential here :)

tests/basic.rs Outdated Show resolved Hide resolved
tests/basic.rs Outdated Show resolved Hide resolved
tests/basic.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation looks good to me :)

examples/basic_wasm_bindgen/src/lib.rs Outdated Show resolved Hide resolved
src/refcount.rs Outdated Show resolved Hide resolved
tests/basic.rs Outdated Show resolved Hide resolved
@Restioson
Copy link
Owner Author

Going to go ahead and merge this now if CI passes

Restioson added 3 commits June 9, 2022 15:15
Technically this is not an MSRV issue, this is an actions + not commiting Cargo.lock issue, since having async_global_executor on 2021 should be fine as long as your compiler is also on 2021, and xtra won't specifically bring in the 2021 edition as it.
@Restioson
Copy link
Owner Author

I'm pretty confident that CI will pass once #80 lands, since the current failure has nothing to do with any code changes in this branch, so I am going to go ahead and merge this now.

@Restioson Restioson merged commit 0e51b00 into master Jun 9, 2022
Restioson added a commit that referenced this pull request Jun 10, 2022
* Add a debug implementation for Address

* fmt

* Ensure debug works for messagechannel & show RC info

* format

* Add test

* Remove refcounter type from struct name

* Ensure debug works for messagechannel & show RC info

* Fix rebase issue

* Turbofish over type annotation (@thomaseizinger)

Thanks to @thomaseizinger!

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>

* Remove unnecessary locking

* rustfmt

* Try fix MSRV issue

* Revert (original change red herring)

Technically this is not an MSRV issue, this is an actions + not commiting Cargo.lock issue, since having async_global_executor on 2021 should be fine as long as your compiler is also on 2021, and xtra won't specifically bring in the 2021 edition as it.

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Restioson added a commit that referenced this pull request Jun 10, 2022
* Add a debug implementation for Address

* fmt

* Ensure debug works for messagechannel & show RC info

* format

* Add test

* Remove refcounter type from struct name

* Ensure debug works for messagechannel & show RC info

* Fix rebase issue

* Turbofish over type annotation (@thomaseizinger)

Thanks to @thomaseizinger!

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>

* Remove unnecessary locking

* rustfmt

* Try fix MSRV issue

* Revert (original change red herring)

Technically this is not an MSRV issue, this is an actions + not commiting Cargo.lock issue, since having async_global_executor on 2021 should be fine as long as your compiler is also on 2021, and xtra won't specifically bring in the 2021 edition as it.

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Restioson added a commit that referenced this pull request Jun 10, 2022
* Addresses are sinks POC (based on flume changes)

* Add a debug implementation for Address (#67)

* Add a debug implementation for Address

* fmt

* Ensure debug works for messagechannel & show RC info

* format

* Add test

* Remove refcounter type from struct name

* Ensure debug works for messagechannel & show RC info

* Fix rebase issue

* Turbofish over type annotation (@thomaseizinger)

Thanks to @thomaseizinger!

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>

* Remove unnecessary locking

* rustfmt

* Try fix MSRV issue

* Revert (original change red herring)

Technically this is not an MSRV issue, this is an actions + not commiting Cargo.lock issue, since having async_global_executor on 2021 should be fine as long as your compiler is also on 2021, and xtra won't specifically bring in the 2021 edition as it.

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>

* Minimal 2021 edition update, MSRV -> 1.56, GHA update (#80)

* Minimal ed2021 update, and MSRV -> 1.56

* Set MSRV in Cargo.toml

* Use minimal versions for all CI runs

* typo

* Update minimum dep for basic_wasm_bindgen

console_error_panic_hook < 0.1.5 was nightly only. This change was needed to make CI with minimum versions pass. Really, it's a little unnecessary for an example to have an MSRV, but to make this CI work with the fewest modifications it's the easiest option.

* Update to HEAD of flume fork

* Sink for Handler<Return = ()> only

* Add example showcasing usage of `Address` as `Sink`

* Add example for sending on an interval based on stream

* Add a debug implementation for Address (#67)

* Add a debug implementation for Address

* fmt

* Ensure debug works for messagechannel & show RC info

* format

* Add test

* Remove refcounter type from struct name

* Ensure debug works for messagechannel & show RC info

* Fix rebase issue

* Turbofish over type annotation (@thomaseizinger)

Thanks to @thomaseizinger!

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>

* Remove unnecessary locking

* rustfmt

* Try fix MSRV issue

* Revert (original change red herring)

Technically this is not an MSRV issue, this is an actions + not commiting Cargo.lock issue, since having async_global_executor on 2021 should be fine as long as your compiler is also on 2021, and xtra won't specifically bring in the 2021 edition as it.

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>

* Addresses are sinks POC (based on flume changes)

* Fmt

* update examples (Address is no longer Sync)

* Fmt

* 1.56 compliance (for CI)

* Update flume to crates.io

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
@Restioson Restioson deleted the debug_address branch June 13, 2022 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Could Address implement fmt::Debug?
2 participants