Skip to content

fix memory leak #837

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

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
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: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants