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

Add outdate, update and upgrade commands #865

Merged
merged 6 commits into from Jul 27, 2020
Merged

Conversation

silvanocerza
Copy link
Contributor

  • 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)
  • What kind of change does this PR introduce?

Add three new commands:

  • outdated
  • update
  • upgrade
  • What is the current behavior?

Those commands don't exist.

  • What is the new behavior?

  • arduino-cli outdated returns a list of cores and libraries that are outdated, showing the currently installed version and the newly available one.

  • arduino-cli update downloads and installs the index for installed cores and libraries if new ones are available.

  • arduino-cli upgrade downloads and installs all available upgrades for installaed cores and libraries.

  • Does this PR introduce a breaking change?

Nope.

  • Other information:

None.


See how to contribute

@silvanocerza silvanocerza added topic: documentation Related to documentation for the project component/CLI labels Jul 24, 2020
@silvanocerza silvanocerza requested a review from a team July 24, 2020 14:29
@silvanocerza silvanocerza self-assigned this Jul 24, 2020
@silvanocerza silvanocerza changed the title Scerza/new commands Adds outdate, update and upgrade commands Jul 24, 2020
@silvanocerza silvanocerza changed the title Adds outdate, update and upgrade commands Add outdate, update and upgrade commands Jul 24, 2020
Copy link
Contributor

@rsora rsora left a comment

Choose a reason for hiding this comment

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

Some nitpick and a functional question for @ubidefeo

assert run_command("core install arduino:samd")
assert run_command("lib install ArduinoJson")

# Verifies only outdate core and library are returned
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Verifies only outdate core and library are returned
# Verifies only outdated cores and libraries are returned

func NewCommand() *cobra.Command {
outdatedCommand := &cobra.Command{
Use: "outdated",
Short: "Lists cores and libraries that can be upgraded\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Short: "Lists cores and libraries that can be upgraded\n",
Short: "Lists cores and libraries that can be upgraded",

This created an empty row in the autocompletion like:

╰─$ ./arduino-cli
board            -- Arduino board commands.
burn-bootloader  -- Upload the bootloader.
cache            -- Arduino cache commands.
compile          -- Compiles Arduino sketches.
completion       -- Generates completion scripts
config           -- Arduino configuration commands.
core             -- Arduino core operations.
daemon           -- Run as a daemon on port 50051
debug            -- Debug Arduino sketches.
help             -- Help about any command
lib              -- Arduino commands about libraries.
outdated         -- Lists cores and libraries that can be upgraded
 
sketch           -- Arduino CLI sketch commands.
update           -- Updates the index of cores and libraries
upgrade          -- Upgrades installed cores and libraries.
upload           -- Upload Arduino sketches.
version          -- Shows version number of Arduino CLI.

}

logrus.Info("Executing `arduino outdated`")

Copy link
Contributor

Choose a reason for hiding this comment

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

To really see all the outdated cores and libraries, should we launch a commands.UpdateIndex() and a UpdateLibrariesIndex() before checking through the local libs and cores? (this is also a functional question for @ubidefeo 🙂 )

Choose a reason for hiding this comment

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

in Homebrew style this command only returns something after an update, bit I like the idea that it implicitly runs arduino-cli update before running

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll change that. 👍

assert run_command("core install arduino:samd")
assert run_command("lib install ArduinoJson")

assert run_command("upgrade")
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be reasonable to add a double check here like you did in the outdated test, checking if no more stuff to upgrade is present after an upgrade WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's kinda problematic for upgrade since basically all the strings contain the latest versions of libraries and cores being upgraded, can't really hardcode it.

I can check oudated commands output though.

@ubidefeo ubidefeo requested review from ubidefeo and rsora July 27, 2020 12:56
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.

@silvanocerza and I went through some small cosmetic/usage changes
I tested and it works beautifully
@rsora can you review/approve?

Copy link
Contributor

@rsora rsora left a comment

Choose a reason for hiding this comment

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

LGTM

@silvanocerza silvanocerza merged commit b8475a0 into master Jul 27, 2020
@silvanocerza silvanocerza deleted the scerza/new-commands branch July 27, 2020 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: documentation Related to documentation for the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants