-
Notifications
You must be signed in to change notification settings - Fork 704
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
feat(artifacts): Add support for helm/chart artifacts #1097
Conversation
d620a17
to
72e9e0e
Compare
@spinnaker/reviewers review, please |
I'm not an official reviewer, but I think there should be some changes to allow the Halyard CLI to manage this configuration. See
Also - features like this should also include a PR to update the official documentation. https://www.spinnaker.io/setup/artifacts/ |
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.
Looks good! As @jfillo noted, you'll want to add the HelmArtifactProviderCommand
so this can be configured via the command line.
The documentation will actually auto-generate once you've added that if you run make
from the halyard-cli
subdirectory; you can just submit those auto-generated docs along with the rest of the changes.
} | ||
|
||
@Override | ||
public void accept(ConfigProblemSetBuilder psBuilder, Validator v) { |
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.
Looks like we could push this up to the base class in ArtifactProvider
. (I realize all the existing accounts have overridden it, but seems like a good opportunity to avoid duplicating the code again.)
72e9e0e
to
3731119
Compare
@eromanova : Looks great, thanks for the changes. |
Issue: spinnaker/spinnaker#3284