Skip to content

fix: add boolean to dispose function call #2341

Merged
BeksOmega merged 2 commits intoRaspberryPiFoundation:masterfrom
sancodes:master
May 7, 2024
Merged

fix: add boolean to dispose function call #2341
BeksOmega merged 2 commits intoRaspberryPiFoundation:masterfrom
sancodes:master

Conversation

@sancodes
Copy link
Contributor

@sancodes sancodes commented May 6, 2024

The basics

The details

Resolves

Fixes #2217

Proposed Changes

  • adds true to the dispose()
  • Screenshot 2024-05-06 at 3 14 45 PM

Reason for Changes

  • adding dispose(true) makes sure the blocks that are not associated with the procedures block remain undeleted

  • Test Coverage

  • Test, I tried to recreate, the blocks being connected to each other, and mimicking the deletion process of procedures child, which should still keep the other blocks intact and undeleted
    Screenshot 2024-05-06 at 3 17 33 PM

My testing logic was:

  • create the def procedure block"type": "procedures_defnoreturn"
    Screenshot 2024-05-06 at 3 26 01 PM

  • create the child block "type": "procedures_callnoreturn"
    Screenshot 2024-05-06 at 3 26 51 PM

  • then basically create two if blocks "type": "controls_if"
    Screenshot 2024-05-06 at 3 27 28 PM

  • then tried to connect those blocks
    Screenshot 2024-05-06 at 3 29 02 PM
    Screenshot 2024-05-06 at 3 29 29 PM

  • I tried to pass in dispose(true), so when the procedure block is deleted, the blocks not associated with procedure block remains intact.

@sancodes sancodes requested a review from a team as a code owner May 6, 2024 22:21
@sancodes sancodes requested review from BeksOmega and removed request for a team May 6, 2024 22:21
Copy link
Contributor

@BeksOmega BeksOmega left a comment

Choose a reason for hiding this comment

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

Hello! Your testing logic is correct but your test code doesn't do what you want it to.

After that fix I'll pull it down to test manually. Then should be good to go =)

@sancodes
Copy link
Contributor Author

sancodes commented May 7, 2024

@BeksOmega applied your suggestions.

@sancodes sancodes changed the title fix: add boolean to dispose function call (#2217) fix: add boolean to dispose function call May 7, 2024
Copy link
Contributor

@BeksOmega BeksOmega left a comment

Choose a reason for hiding this comment

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

Tested locally and this LGTM =)

Thank you for the fix!!

@BeksOmega BeksOmega merged commit 161c0d1 into RaspberryPiFoundation:master May 7, 2024
@sancodes
Copy link
Contributor Author

sancodes commented May 7, 2024

@BeksOmega Thank you :)

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.

Disposing of a procedure definition disposes caller *and* next blocks

2 participants

Comments