Skip to content
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

refactor: Made command functions to access PackageManager unavailable from public API #2335

Merged
merged 7 commits into from
Oct 19, 2023

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Sep 22, 2023

Please check if the PR fulfills these requirements

See how to contribute

  • 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)
  • configuration.schema.json updated if new parameters are added.

What kind of change does this PR introduce?

This refactoring aims to make the following functions of the commands package private:

commands.GetPackageManagerExplorer(...)
commands.GetPackageManager(...)
commands.GetLibrariesManager(...)

This change should clarify that the arduino package must not be used from the cli package (to avoid bypassing the gRPC API).
It also highlights where incorrect access to the arduino package is made, because those places are now reported as compile-time errors:

package github.com/arduino/arduino-cli
	imports github.com/arduino/arduino-cli/internal/cli
	imports github.com/arduino/arduino-cli/internal/cli/board
	imports github.com/arduino/arduino-cli/internal/cli/arguments
	internal/cli/arguments/completion.go:23:2: use of internal package github.com/arduino/arduino-cli/commands/internal/instances not allowed
package github.com/arduino/arduino-cli
	imports github.com/arduino/arduino-cli/internal/cli
	imports github.com/arduino/arduino-cli/internal/cli/board
	imports github.com/arduino/arduino-cli/internal/cli/arguments
	internal/cli/arguments/port.go:24:2: use of internal package github.com/arduino/arduino-cli/commands/internal/instances not allowed

The first one tries to list the available upload protocols. The second one performs a board watch to get available ports.

Both have been updated to use only the public gRPC API.

What is the current behavior?

No changes.

What is the new behavior?

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

No

Other information

Once, completed this PR should fix #1948

@cmaglie cmaglie self-assigned this Sep 22, 2023
@cmaglie cmaglie added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Sep 25, 2023
Removed the shorthand of the InstanceCommand interface.
This makes the API more coherent among the `commands` instance handling.
This change highlights the incorrect PackageManager access in 'cli'
package. Now those are errors:

package github.com/arduino/arduino-cli
	imports github.com/arduino/arduino-cli/internal/cli
	imports github.com/arduino/arduino-cli/internal/cli/board
	imports github.com/arduino/arduino-cli/internal/cli/arguments
	internal/cli/arguments/completion.go:23:2: use of internal package github.com/arduino/arduino-cli/commands/internal/instances not allowed
package github.com/arduino/arduino-cli
	imports github.com/arduino/arduino-cli/internal/cli
	imports github.com/arduino/arduino-cli/internal/cli/board
	imports github.com/arduino/arduino-cli/internal/cli/arguments
	internal/cli/arguments/port.go:24:2: use of internal package github.com/arduino/arduino-cli/commands/internal/instances not allowed
@cmaglie cmaglie changed the title refactor: Made command ways to access PackageManager unavailable from public API refactor: Made command functions to access PackageManager unavailable from public API Oct 18, 2023
Previously the function GetConnectedBoards() counter-intuitively
returned a list of port address. Now it has been reneamed
GetAvailablePorts() and returns the full Port object that is mapped into
an array of Addresses or into an array of Prorocols based on the
auto-completion request.
@cmaglie cmaglie added this to the Arduino CLI v1.0.0 milestone Oct 18, 2023
@codecov
Copy link

codecov bot commented Oct 18, 2023

Codecov Report

Attention: 45 lines in your changes are missing coverage. Please review.

Comparison is base (1e51cdc) 62.82% compared to head (6c6db14) 62.77%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2335      +/-   ##
==========================================
- Coverage   62.82%   62.77%   -0.06%     
==========================================
  Files         204      205       +1     
  Lines       19311    19283      -28     
==========================================
- Hits        12132    12104      -28     
  Misses       6119     6119              
  Partials     1060     1060              
Flag Coverage Δ
unit 62.77% <68.30%> (-0.06%) ⬇️

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

Files Coverage Δ
commands/board/list.go 58.70% <100.00%> (ø)
commands/board/listall.go 91.13% <100.00%> (ø)
commands/compile/compile.go 66.78% <100.00%> (ø)
commands/core/download.go 57.14% <100.00%> (ø)
commands/core/install.go 71.42% <100.00%> (ø)
commands/core/list.go 86.95% <100.00%> (ø)
commands/core/search.go 84.21% <100.00%> (ø)
commands/core/uninstall.go 50.00% <100.00%> (ø)
commands/core/upgrade.go 80.64% <100.00%> (ø)
commands/debug/debug_info.go 66.86% <100.00%> (ø)
... and 19 more

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

@cmaglie cmaglie marked this pull request as ready for review October 18, 2023 19:45
@umbynos umbynos removed this from the Arduino CLI v1.0.0 milestone Oct 19, 2023
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.

@cmaglie cmaglie merged commit 4b2a32b into arduino:master Oct 19, 2023
96 checks passed
@cmaglie cmaglie deleted the refactor_commands_internals branch October 19, 2023 13:35
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.

cli code refactoring: argument parsing/handling should not access the arduino package directly
3 participants