Applied suggestions from code review

Co-authored-by: per1234 <[email protected]>

Author: cmaglie

arduino/resources/download.go

@@ -25,7 +25,7 @@ import (
 	"go.bug.st/downloader/v2"
 )
 
-// Download performs a download loop using the provided downloader.Downloader.
+// Download performs a download loop using the provided downloader.Config.
 // Messages are passed back to the DownloadProgressCB using label as text for the File field.
 func (r *DownloadResource) Download(downloadDir *paths.Path, config *downloader.Config, label string, downloadCB rpc.DownloadProgressCB) error {
 	path, err := r.ArchivePath(downloadDir)

arduino/resources/index.go

@@ -28,7 +28,7 @@ import (
 	"go.bug.st/downloader/v2"
 )
 
-// IndexResource is a reference to a package_index.json
+// IndexResource is a reference to an index file URL with an optional signature.
 type IndexResource struct {
 	URL          *url.URL
 	SignatureURL *url.URL

configuration/network.go

@@ -24,7 +24,7 @@ import (
 	"github.com/spf13/viper"
 )
 
-// UserAgent returns the user agent for the cli (mainly used by http clients)
+// UserAgent returns the user agent (mainly used by HTTP clients)
 func UserAgent(settings *viper.Viper) string {
 	subComponent := settings.GetString("network.user_agent_ext")
 	if subComponent != "" {
@@ -39,7 +39,7 @@ func UserAgent(settings *viper.Viper) string {
 		globals.VersionInfo.Commit)
 }
 
-// NetworkProxy returns the proxy configuration (mainly used by http clients)
+// NetworkProxy returns the proxy configuration (mainly used by HTTP clients)
 func NetworkProxy(settings *viper.Viper) (*url.URL, error) {
 	if !settings.IsSet("network.proxy") {
 		return nil, nil

docs/UPGRADING.md

@@ -4,9 +4,15 @@ Here you can find a list of migration guides to handle breaking changes between
 
 ## 0.22.0
 
-### package `github.com/arduino/arduino-cli/httpclient` has been moved to `github.com/arduino/arduino-cli/arduino/httpclient`
+### The content of package `github.com/arduino/arduino-cli/httpclient` has been moved to a different path
 
-The old import must be updated to the new one.
+In particular:
+
+- `UserAgent` and `NetworkProxy` have been moved to `github.com/arduino/arduino-cli/configuration`
+- the remainder of the package `github.com/arduino/arduino-cli/httpclient` has been moved to
+  `github.com/arduino/arduino-cli/arduino/httpclient`
+
+The old imports must be updated according to the list above.
 
 ### `commands.DownloadProgressCB` and `commands.TaskProgressCB` have been moved to package `github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1`
 
@@ -20,7 +26,7 @@ All references to this function must be updated with the new import.
 
 The old function must be replaced by the new one that is much more versatile.
 
-### `packagemanager.PackageManager.DownloadToolRelease`, `packagemanager.PackageManager.DownloadPlatformRelease`, and `resources.DownloadResource.Download` functions now require `label` and `progressCB` parameters
+### `packagemanager.PackageManager.DownloadToolRelease`, `packagemanager.PackageManager.DownloadPlatformRelease`, and `resources.DownloadResource.Download` functions change signature and behaviour
 
 The following functions:
 
@@ -30,22 +36,32 @@ func (pm *PackageManager) DownloadPlatformRelease(platform *cores.PlatformReleas
 func (r *DownloadResource) Download(downloadDir *paths.Path, config *downloader.Config) (*downloader.Downloader, error)
 ```
 
-now requires a progress callback:
+now requires a label and a progress callback parameter, do not return the `Downloader` object anymore, and they
+automatically handles the download internally:
 
 ```go
 func (pm *PackageManager) DownloadToolRelease(tool *cores.ToolRelease, config *downloader.Config, label string, progressCB rpc.DownloadProgressCB) error
 func (pm *PackageManager) DownloadPlatformRelease(platform *cores.PlatformRelease, config *downloader.Config, label string, progressCB rpc.DownloadProgressCB) error
 func (r *DownloadResource) Download(downloadDir *paths.Path, config *downloader.Config, label string, downloadCB rpc.DownloadProgressCB) error
 ```
 
-The new parameters must be added to legacy code. If progress reports are not needed an empty stub for `label` and
-`progressCB` must be provided, for example:
+The new progress parameters must be added to legacy code, if progress reports are not needed an empty stub for `label`
+and `progressCB` must be provided. There is no more need to execute the `downloader.Run()` or
+`downloader.RunAndPoll(...)` method.
+
+For example, the old legacy code like:
 
 ```go
-err := pm.DownloadPlatformRelease(platformToDownload, config)
+downloader, err := pm.DownloadPlatformRelease(platformToDownload, config)
+if err != nil {
+    ...
+}
+if err := downloader.Run(); err != nil {
+    ...
+}
 ```
 
-becomes:
+may be ported to the new version as:
 
 ```go
 err := pm.DownloadPlatformRelease(platformToDownload, config, "", func(progress *rpc.DownloadProgress) {})