Skip to content

Add __del__ method to BusABC#1489

Merged
zariiii9003 merged 25 commits into
hardbyte:developfrom
Tbruno25:tj/1488_use-del
Mar 30, 2023
Merged

Add __del__ method to BusABC#1489
zariiii9003 merged 25 commits into
hardbyte:developfrom
Tbruno25:tj/1488_use-del

Conversation

@Tbruno25
Copy link
Copy Markdown
Contributor

@Tbruno25 Tbruno25 commented Jan 17, 2023

Initial PR to address #1448

Closes #1448

Comment thread can/bus.py Outdated
@Tbruno25 Tbruno25 force-pushed the tj/1488_use-del branch 2 times, most recently from 43d6418 to 7f028d6 Compare January 17, 2023 04:54
@felixdivo felixdivo added this to the Next Release milestone Jan 22, 2023
@felixdivo felixdivo added the api label Jan 22, 2023
Comment thread can/bus.py Outdated
Comment thread can/bus.py Outdated
TJ Bruno and others added 7 commits January 23, 2023 14:38
Co-authored-by: Felix Divo <4403130+felixdivo@users.noreply.github.com>
Co-authored-by: Felix Divo <4403130+felixdivo@users.noreply.github.com>
Comment thread can/bus.py Outdated
@felixdivo felixdivo added the bug label Jan 24, 2023
@felixdivo
Copy link
Copy Markdown
Collaborator

I added the bug label since now, for example, etas properly calls stop_all_periodic_tasks() on shutdown.

Tbruno25 and others added 3 commits January 25, 2023 11:18
Co-authored-by: Felix Divo <4403130+felixdivo@users.noreply.github.com>
Copy link
Copy Markdown
Collaborator

@felixdivo felixdivo left a comment

Choose a reason for hiding this comment

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

This is great! I'll have to check if pytest.mark.filterwarnings is really a good idea, and not hiding any other problems. Else, this can be merged.

@felixdivo
Copy link
Copy Markdown
Collaborator

I'll have to check if pytest.mark.filterwarnings is really a good idea

Hey @Tbruno25, would you mind commenting on whether #1519 is a good idea?

@zariiii9003
Copy link
Copy Markdown
Collaborator

@felixdivo i cherry-picked your changes and fixed the AttributeError. Would you like to take another look?

Copy link
Copy Markdown
Collaborator

@felixdivo felixdivo 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 this looks good. Didn't test it locally, but you probably did and we have good unit tests on this. Nice work! 🥳

@zariiii9003 zariiii9003 merged commit 7855da1 into hardbyte:develop Mar 30, 2023
@Tbruno25 Tbruno25 deleted the tj/1488_use-del branch March 30, 2023 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider using __del__

3 participants