-
Notifications
You must be signed in to change notification settings - Fork 20
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
Allow installation of an app without specifying version #309
Conversation
Does the Apps service already supports this? |
No. I have a PR pending. |
It's good practice to put a "Depends on" with a link to the PR when that's the case. |
This is available in apps@0.11.0-beta |
src/modules/apps/install.ts
Outdated
@@ -12,7 +13,16 @@ const installApps = async (apps: string[], reg: string): Promise<void> => { | |||
if (apps.length === 0) { | |||
return | |||
} | |||
const app = validateApp(head(apps)) | |||
var app; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use let
when you need to rebind the variable, or const
(preferably) when you don't need to rebind the variable.
We don't use var
anymore.
src/modules/apps/install.ts
Outdated
} | ||
app = validateApp(head(apps), false) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get why you validate the app twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I attacked the wrong problem. Instead of fixing the validation, I used it twice with different arguments so it would give me the expected result. Now I fixed it.
daabf02
to
2f4be3e
Compare
@tamorim, PTAL |
@igorframos LGTM 👍 |
What is the purpose of this pull request?
Do not reject an app ID for installation if it does not have a version.
What problem is this solving?
Sometimes we just do not care about the version of an app we are installing, we just want the last one. Usually this is done by allowing users to not specify a version when installing the app.
How should this be manually tested?
Install an app using version and see it work as always.
Try to install an app without specifying version and see either:
Screenshots or example usage
Types of changes