Skip to content

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Jul 20, 2022

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)

What kind of change does this PR introduce?
Removes a TON of code-duplication. Makes a couple of fields in PackageManager private.

Does this PR introduce a breaking change, and is titled accordingly?
Yes

Other information:
This PR is part of a bigger refactoring to solve concurrency problems in PackageManager, it started by trying to make the PackageManager fields private, but making the Log field private started a massive chain reaction that lead to this PR.

@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Jul 22, 2022
@cmaglie cmaglie self-assigned this Jul 26, 2022
@cmaglie cmaglie requested review from per1234 and umbynos August 4, 2022 10:27
@codecov
Copy link

codecov bot commented Aug 5, 2022

Codecov Report

Merging #1809 (851dfc3) into master (2dd8976) will increase coverage by 0.37%.
The diff coverage is 12.54%.

@@            Coverage Diff             @@
##           master    #1809      +/-   ##
==========================================
+ Coverage   35.50%   35.87%   +0.37%     
==========================================
  Files         230      231       +1     
  Lines       19532    19324     -208     
==========================================
- Hits         6934     6933       -1     
+ Misses      11773    11567     -206     
+ Partials      825      824       -1     
Flag Coverage Δ
unit 35.87% <12.54%> (+0.37%) ⬆️

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

Impacted Files Coverage Δ
arduino/cores/packagemanager/profiles.go 0.00% <0.00%> (ø)
cli/outdated/outdated.go 0.00% <0.00%> (ø)
cli/update/update.go 0.00% <0.00%> (ø)
cli/upgrade/upgrade.go 0.00% <0.00%> (ø)
commands/core/download.go 0.00% <0.00%> (ø)
commands/core/install.go 0.00% <0.00%> (ø)
commands/core/uninstall.go 0.00% <0.00%> (ø)
commands/core/upgrade.go 0.00% <0.00%> (ø)
commands/daemon/daemon.go 0.00% <0.00%> (ø)
commands/instances.go 39.45% <0.00%> (+15.28%) ⬆️
... and 7 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@cmaglie cmaglie force-pushed the downloader_installer_refactor branch from 65b3621 to 851dfc3 Compare August 5, 2022 10:38
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

Excellent tidying up of the code happening here.

Thanks Cristian!

@cmaglie cmaglie merged commit c7163b7 into arduino:master Aug 8, 2022
@cmaglie cmaglie deleted the downloader_installer_refactor branch August 8, 2022 17:06
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.

3 participants