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

docs: Improve documentation for --publish flag #2024

Merged
merged 10 commits into from
Jan 18, 2024

Conversation

rizul2108
Copy link
Contributor

Summary

I am changing the help-text for --publish flag as it was not properly descriptive earlier.

Resolves #1994

@rizul2108 rizul2108 requested review from a team as code owners January 13, 2024 18:14
@github-actions github-actions bot added this to the 0.33.0 milestone Jan 13, 2024
@github-actions github-actions bot added the type/enhancement Issue that requests a new feature or improvement. label Jan 13, 2024
Signed-off-by: Rizul Gupta <mail2rizul@gmail.com>
Copy link

codecov bot commented Jan 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5dffab6) 79.50% compared to head (d27f8d8) 79.50%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2024   +/-   ##
=======================================
  Coverage   79.50%   79.50%           
=======================================
  Files         175      175           
  Lines       13114    13114           
=======================================
  Hits        10425    10425           
  Misses       2022     2022           
  Partials      667      667           
Flag Coverage Δ
os_linux 78.42% <100.00%> (ø)
os_macos 76.24% <100.00%> (ø)
os_windows 78.88% <100.00%> (ø)
unit 79.50% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Member

@natalieparellano natalieparellano left a comment

Choose a reason for hiding this comment

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

@rizul2108 - thanks a lot for this. I left some suggestions. I avoided using making it available for deployment and use by others because that is sort of subjective, users can have private registries with access controls and we don't want to imply that the image will automatically be made "public". LMK what you think :)

@rizul2108
Copy link
Contributor Author

@rizul2108 - thanks a lot for this. I left some suggestions. I avoided using making it available for deployment and use by others because that is sort of subjective, users can have private registries with access controls and we don't want to imply that the image will automatically be made "public". LMK what you think :)

Yeah I missed that point that it can be published to private registries also.I agree about that point.

rizul2108 and others added 7 commits January 17, 2024 23:00
Co-authored-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Rizul Gupta <112455393+rizul2108@users.noreply.github.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Rizul Gupta <112455393+rizul2108@users.noreply.github.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Rizul Gupta <112455393+rizul2108@users.noreply.github.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Rizul Gupta <112455393+rizul2108@users.noreply.github.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Rizul Gupta <112455393+rizul2108@users.noreply.github.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Rizul Gupta <112455393+rizul2108@users.noreply.github.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Rizul Gupta <112455393+rizul2108@users.noreply.github.com>
@rizul2108
Copy link
Contributor Author

@natalieparellano can you please review now I committed your suggested changes

@jjbustamante
Copy link
Member

@rizul2108 thank you so much for this contribution!!

Copy link
Member

@jjbustamante jjbustamante left a comment

Choose a reason for hiding this comment

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

LGTM

@jjbustamante jjbustamante merged commit 60922df into buildpacks:main Jan 18, 2024
18 checks passed
@@ -241,7 +241,7 @@ func buildCommandFlags(cmd *cobra.Command, buildFlags *BuildFlags, cfg config.Co
cmd.Flags().StringVar(&buildFlags.Network, "network", "", "Connect detect and build containers to network")
cmd.Flags().StringArrayVar(&buildFlags.PreBuildpacks, "pre-buildpack", []string{}, "Buildpacks to prepend to the groups in the builder's order")
cmd.Flags().StringArrayVar(&buildFlags.PostBuildpacks, "post-buildpack", []string{}, "Buildpacks to append to the groups in the builder's order")
cmd.Flags().BoolVar(&buildFlags.Publish, "publish", false, "Publish to registry")
cmd.Flags().BoolVar(&buildFlags.Publish, "publish", false, "Publish the application image directly to the container registry specified in <image-name>, instead of the daemon. The run image must also reside in the registry.")

Choose a reason for hiding this comment

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

Why do you say that the The run image must also reside in the registry ? AFAIK pack build or pack builder build and push image according to toml file definition and application path but I never see something about to have a run image .... @jjbustamante

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve documentation for --publish flag
4 participants