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

Added setup-helm : Install helm binary #1

Merged
merged 3 commits into from Oct 15, 2019

Conversation

Anumita
Copy link
Contributor

@Anumita Anumita commented Oct 4, 2019

No description provided.

const helmpath = findHelm(cachedToolpath);
if (!helmpath) {

}
Copy link
Contributor

Choose a reason for hiding this comment

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

If ! throw some warning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

function findHelm(rootFolder: string): string {
fs.chmodSync(rootFolder, '777');
var filelist: string[] = [];
walkSync(rootFolder, filelist, helmToolName + getExecutableExtension());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need this? You could use tl.find?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its not there in tool-cache
the find function in tool-cache is only for tools and not for paths

src/run.ts Outdated

var walkSync = function(dir, filelist, fileToFind) {
var fs = fs || require('fs'),
files = fs.readdirSync(dir);
Copy link
Contributor

Choose a reason for hiding this comment

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

Split it into 2 lines, also add the require statement globally. Otherwise, you'd be bringing it into memory in every stack call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -0,0 +1,122 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not required?

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 was mentioned in the workitem that ariel sent for compliance

@@ -0,0 +1,15 @@
name: 'Helm tool installer'
description: 'Install a specific version of helm binary. Acceptable values are latest or any semantic version string like 1.15.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

or latest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

already mentioned latest

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.

None yet

2 participants