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

Warn when some templates can't be upgraded #1147

Merged
merged 1 commit into from
Feb 15, 2023

Conversation

itowlson
Copy link
Contributor

Originally, spin templates upgrade ignored templates with no origin info, because there's not much it can do about them.

However, this was a poor initial experience because existing installs would not have origin info. So now, if we detect that no templates have origin info (and so cannot be upgraded), we print a warning that tells people how to upgrade the old way.

But this is still a suboptimal experience because a user could, say, old-skool upgrade the Spin repo templates and forget about the JS SDK repo. Now the upgrade command would offer to upgrade the Spin repo, but mysteriously not show the JS repo, and there would be no clue as to why.

This PR adds YET MOAR verbiage to the flood. In the situation above, the user will now see:

$ spin templates upgrade
Spin could not determine where the following templates were installed from:
- http-js
- http-ts
To upgrade them, run `spin templates install --upgrade` with the --git or --dir option

The following template repositories can be automatically upgraded:
> [ ] https://github.com/fermyon/spin (at spin/templates/v0.8)

(The additional "can be automatically upgraded" line is shown only if there was a warning; it is there to separate the warning from the selector, and make the context of the selector clear.)

The hope is that, after the initial experience, users will not see this verbiage again. However, users with templates installed from local directories will see it each time. Hopefully that is edge case enough not to be a significant issue - and if it is we can refine it over time.

Signed-off-by: itowlson <ivan.towlson@fermyon.com>
@itowlson itowlson force-pushed the templates-upgrade-skippity-hop branch from b0968cd to 6f268b4 Compare February 14, 2023 21:56
}
eprintln!("To upgrade them, run `spin templates install --upgrade` with the --git or --dir option");
eprintln!();
if !self.all {
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a possibility of this message ("The following template repositories can be automatically upgraded:") printing and then there not being any with an origin to display after?

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 don't believe so. This is ensured by the sources.is_empty() block starting on line 244 - it is guaranteed to early exit on 255.

I have no qualms about adding a belt and braces check here, though, if you think it's worth it?

(Well no qualms except the THIRD RUN through CI but okay that's not so much a qualm as a pet peeve.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thats perfect as it is. Didnt take a deep look, so thanks for pointing out the check.

@itowlson itowlson merged commit 9531921 into fermyon:main Feb 15, 2023
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.

2 participants