Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix memory leak #837

Merged
merged 5 commits into from
Oct 4, 2023
Merged

fix memory leak #837

merged 5 commits into from
Oct 4, 2023

Conversation

thechampagne
Copy link
Contributor

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • Tests for the changes have been added (for bug fixes / features)
  • What kind of change does this PR introduce?
  • What is the current behavior?
  • What is the new behavior?
  • Does this PR introduce a breaking change?
  • Other information:

@CLAassistant
Copy link

CLAassistant commented Sep 25, 2023

CLA assistant check
All committers have signed the CLA.

@codecov-commenter
Copy link

codecov-commenter commented Sep 25, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (e740ad9) 17.76% compared to head (f19f336) 18.37%.
Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #837      +/-   ##
==========================================
+ Coverage   17.76%   18.37%   +0.60%     
==========================================
  Files          53       53              
  Lines        4103     4120      +17     
==========================================
+ Hits          729      757      +28     
+ Misses       3270     3257      -13     
- Partials      104      106       +2     
Flag Coverage Δ
unit 18.37% <ø> (+0.60%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 8 files with indirect coverage changes

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

@per1234 per1234 added topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project labels Sep 25, 2023
@umbynos
Copy link
Contributor

umbynos commented Sep 25, 2023

Hello @thechampagne, thanks for your contribution! Could you please add a bit more context to this? It would make the review process a lot faster.
It seems that the CI is failing because it cannot build on macos. Could you please take a look at it?

@umbynos umbynos self-requested a review September 25, 2023 10:12
@umbynos
Copy link
Contributor

umbynos commented Oct 4, 2023

Hello @thechampagne, one last thing, and I think we are ready to merge: could you please run go fmt github.com/arduino/arduino-create-agent/certificates github.com/arduino/arduino-create-agent/systray. It seems that the CI is failing

Copy link
Contributor

@umbynos umbynos left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your contribution!

Copy link

@ubidefeo ubidefeo left a comment

Choose a reason for hiding this comment

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

I have tested this on Mac OS Ventura.
The app is able to generate certificates once the password is correctly entered by the user.
(it prompts for it)

@umbynos umbynos merged commit bf32bad into arduino:main Oct 4, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants