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

Only push published chart if --push specified #162

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

manics
Copy link
Member

@manics manics commented Jun 10, 2022

It's useful when developing a new chart to be able to test the publish chart functionality without pushing the gh-pages branch.

I'm using the existing --push argument since it doesn't seem like there's a use for pushing the chart unless the images are already pushed, though it could be a separate argument instead?

@consideRatio
Copy link
Member

consideRatio commented Jun 10, 2022

Hmmm...

We have the following CLI flags.

  --push                Push built images to their image registries, but not
                        if it would replace an existing image.
  --force-push          Push built images to their image registries,
                        regardless if it would replace an existing image.
  --publish-chart       Package a Helm chart and publish it to a Helm chart
                        registry constructed with a GitHub git repository and
                        GitHub pages, but not if it would replace an existing
                        chart version.
  --force-publish-chart
                        Package a Helm chart and publish it to a Helm chart
                        registry constructed with a GitHub git repository and
                        GitHub pages, regardless if it would replace an
                        existing chart version

What are you suggesting? Are you looking for an option to package but not publish the chart?

@manics
Copy link
Member Author

manics commented Jun 10, 2022

What are you suggesting? Are you looking for an option to package but not publish the chart?

Yes! It's useful during development of a new chart, and also when developing new chartpress features.

@consideRatio
Copy link
Member

I'm hesitant about this PR as it is, thinking that the implication of what the --push flag is stretched too far to be comprehensible for users and maintainers. While the name --push is general, it means to me --push-images and should be treated as that.

I'm using the existing --push argument since it doesn't seem like there's a use for pushing the chart unless the images are already pushed, though it could be a separate argument instead?

Thanks for highlighting this assumption! You can have a chart that doesn't include images to be built. Due to that, this is a breaking change because it can make sense for that chart to do --publish-chart without --push.

I think having a --package-chart flag, which is implied by --publish-chart/--force-publish-chart, would be the way to go for this.

@consideRatio
Copy link
Member

I propose we implement this via --package-chart instead, what do you think @manics?

--package-chart
--publish-chart        # implies --package-chart
--force-publish-chart  # implies --publish-chart

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