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 meadow update command. #576

Closed
wants to merge 1 commit into from
Closed

Conversation

CartBlanche
Copy link
Collaborator

@CartBlanche CartBlanche commented Jun 6, 2024

Command to reduce customer paper cuts when needing to update Meadow.CLI.
It is easier for customers to remember meadow update, which will update to the latest version, rather than remembering:

> dotnet tool update WildernessLabs.Meadow.CLI --global

We could also allow them to roll back to specific versions, should something break or become unstable in vLatest. Obviously not too far back as then meadow update wouldn't be available.

Tested on:

  • Windows
  • Mac
  • Linux

When run on Mac and Linux this is the output:

16:36:35 ~/Downloads $ meadow update
Updating WildernessLabs.Meadow.CLI
This will initially uninstall WildernessLabs.Meadow.CLI and will then install version vLatest
Please wait about 10s to allow the above steps to complete.
16:36:50 ~/Downloads $ Tool 'wildernesslabs.meadow.cli' (version '2.0.47') was successfully uninstalled.
Skipping NuGet package signature verification.
You can invoke the tool using the following command: meadow
Tool 'wildernesslabs.meadow.cli' (version '2.0.48') was successfully installed.

@CartBlanche CartBlanche self-assigned this Jun 6, 2024
@CartBlanche CartBlanche marked this pull request as draft June 6, 2024 16:56
Copy link
Contributor

@adrianstevens adrianstevens left a comment

Choose a reason for hiding this comment

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

It doesn't make any sense to wrap another CLI call in our CLI

@CartBlanche
Copy link
Collaborator Author

It doesn't make any sense to wrap another CLI call in our CLI

I've always thought that the idea of wrapping this particular CLI call (or other complex CLI calls) is to minimise the paper cuts our customers encounter when using the Meadow stack, by having to remember the long MS command as well as the correct parameters required. Also the meadow update was a feature @bryancostanich asked if it was possible, in a thread on Slack (link it today's weeklies).

We already simplify the lives of our customers with our own CLI wrapper of the dfu-util CLI here -
https://github.com/WildernessLabs/Meadow.CLI/blob/develop/Source/v2/Meadow.Dfu/DfuUtils.cs
Where we both automate the installation of dfu-util with meadow install dfu-util and when we call meadow firmware write. As well as on Mac with a wrapper around ioreg CLI command. So I'm not deviating from what we already provide for our customers.

It seems easier for customers to remember meadow update, which will update to the latest version, rather than remembering:

> dotnet tool update WildernessLabs.Meadow.CLI --global

We could also allow them to roll back to specific versions, should something break or become unstable in vLatest. Obviously not too far back as then meadow update wouldn't be available.

@CartBlanche CartBlanche force-pushed the dominique-MeadowUpdate branch from 1aecaf3 to f2dd343 Compare June 17, 2024 13:56
@CartBlanche CartBlanche marked this pull request as ready for review June 17, 2024 14:00
@CartBlanche CartBlanche force-pushed the dominique-MeadowUpdate branch from f2dd343 to 5bbcff1 Compare June 19, 2024 20:37
@CartBlanche CartBlanche force-pushed the dominique-MeadowUpdate branch from 5bbcff1 to ad9fa9c Compare July 2, 2024 13:49
Copy link
Contributor

@adrianstevens adrianstevens left a comment

Choose a reason for hiding this comment

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

Let's make the updater and stand-alone app. I don't think it needs to be called from the Meadow.CLI console app since it's already it's own app.

@CartBlanche
Copy link
Collaborator Author

@adrianstevens just to be clear the Meadow.CLI needs to shell out then shutdown for the updater app to actually then perform the update. So are you suggesting that the updater app be in it's own repo or something else??

@@ -88,7 +88,7 @@ jobs:
run: dotnet build main/MeadowCLI.Classic.sln /p:Configuration=Release

- name: Upload nuget Artifacts for internal testing
uses: actions/upload-artifact@v2
uses: actions/upload-artifact@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

Updates to the build actions should not be in with code changes. If this had been a stand-along PR, it would have already been merged.

@ctacke
Copy link
Contributor

ctacke commented Sep 24, 2024

I'm also in the camp of "this is not a good idea". If we're simply trying to make it easier a user who forgets the command, why not just echo out the proper command for them? Creating a new command that creates a new process and calls some other existing CLI seems clunky AF

@bryancostanich
Copy link
Contributor

This was my fault. Users kept running into issues with the update and i asked for this feature but didn't realize what a cluster it would be.

@adrianstevens adrianstevens deleted the dominique-MeadowUpdate branch December 14, 2024 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants