Skip to content

Conversation

2bndy5
Copy link

@2bndy5 2bndy5 commented Feb 24, 2024

resolves #90

Goals

  • reduce the visual noise in a PR thread.
  • allow only 1 comment to be posted/updated to reduce the notifications triggered from a PR sync event.

What does it do?

  • adds a new input update-comment that defaults to false (for current behavior)
  • when update-comment is set to true, the first report comment found is updated. Furthermore, if other report comments are found in the same thread, they will be deleted.
    • bot comments are now prefixed with an HTML comment (<!-- arduino/report-size-deltas -->) to distinguish bot comments from any user comment that also starts with **Memory usage change @ . This means no user comments will be accidentally deleted or updated.

Other changes

I prefer to use a local venv for developing on python-based projects. So, I had to add some typical names used for a local venv located at repo root to .gitignore, .flake8, and .prettierignore.

I have made sure that all unit tests still pass and added one unit test for the new functionality.

@codecov-commenter
Copy link

codecov-commenter commented Feb 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (0ca09e5) to head (13c9484).

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #91   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines          752       776   +24     
=========================================
+ Hits           752       776   +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@2bndy5 2bndy5 force-pushed the update-comment branch 2 times, most recently from 6364a8c to 78a9da8 Compare March 15, 2024 21:34
@2bndy5
Copy link
Author

2bndy5 commented Mar 26, 2024

ping @per1234

@2bndy5
Copy link
Author

2bndy5 commented Sep 18, 2024

I've been keeping my fork in sync with upstream. I've also been using these changes for all nRF24 org repos (since I opened this PR), and I haven't seen any unexpected behavior so far.

Not sure what I need to do to get this reviewed (by admins). It's been idling for over half a year. This repo hasn't been getting much attention lately 😞 .

@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Feb 1, 2025
@2bndy5
Copy link
Author

2bndy5 commented May 28, 2025

I removed the change about required python version. I was able to setup a local venv with python v3.11 using uv.

@per1234 Please review this. I've been rebasing my branch for over a year.

2bndy5 added 5 commits July 10, 2025 14:16
If no comment exists, then simply post a new comment. If more than 1 comment found, then delete all but last one and update the last one.
distutils has been deprecated for years; use shutil instead

add unit test for `get_previous_comment()`
defaults to false for old behavior
update tests accordingly

replaces `get_previous_comment()` which was almost identical to `report_exists()`
avoids treating user comments as a bot comment
@2bndy5
Copy link
Author

2bndy5 commented Jul 19, 2025

Ported to rust

I'm officially done with trying to support/improve this action. I have ported this action to rust and will continue to improve that instead. See 2bndy5/arduino-report-size-deltas. I've outlined my rationale and the state of feature parity in the README.

My port uses this "Update comment" feature automatically. I also took some liberty in the collapsed "full detailed report"; it now uses 1 table per board instead of a 1 nearly incomprehensible table.

Sample from 2bndy5/arduino-report-size-deltas

Memory usage change @ 651f05f4d4aca30ac359e972c01568f873112d43

Board Flash % RAM for global variables %
arduino:avr:leonardo 💚 -40 - 0 -0.14 - 0 💚 -16 - 0 -0.62 - 0
arduino:avr:nano 0 - 0 0 - 0 0 - 0 0 - 0
arduino:sam:arduino_due_x 0 - 0 0 - 0 N/A N/A
arduino:samd:mkrzero ❔ -28 - +84 -0.01 - +0.03 🔺 0 - +32 0 - +0.1
Click for full report per board

arduino:avr:leonardo

Sketch Flash % RAM for global variables %
examples/MIDIUSB_clock 0 0 0 0
examples/MIDIUSB_loop 0 0 0 0
examples/MIDIUSB_read 0 0 0 0
examples/MIDIUSB_write -40 -0.14 -16 -0.62

arduino:avr:nano

Sketch Flash % RAM for global variables %
examples/helloworld_rx 0 0 0 0
examples/helloworld_rx_advanced 0 0 0 0
examples/helloworld_tx_advanced 0 0 0 0
examples/helloworld_tx 0 0 0 0
examples/Network_Priority_RX 0 0 0 0
examples/Network_Priority_TX 0 0 0 0

arduino:sam:arduino_due_x

Sketch Flash % RAM for global variables %
examples/MIDIUSB_clock 0 0 N/A N/A
examples/MIDIUSB_loop 0 0 N/A N/A
examples/MIDIUSB_read 0 0 N/A N/A
examples/MIDIUSB_write 0 0 N/A N/A

arduino:samd:mkrzero

Sketch Flash % RAM for global variables %
examples/MIDIUSB_clock +84 +0.03 0 0
examples/MIDIUSB_loop 0 0 0 0
examples/MIDIUSB_read -28 -0.01 0 0
examples/MIDIUSB_write 0 0 +32 +0.1

@2bndy5 2bndy5 closed this Aug 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

update comment if it exists
4 participants