pack subcommands #93

jromero · 2020-07-08T19:02:34Z

Readable

text/0000-pack-subcommands.md

hone · 2020-07-15T17:27:58Z

text/0000-pack-subcommands.md

+[alternatives]: #alternatives
+
+- Keep it as it is.
+- Some [Config](#config) subcommands could be more intertwined with their respective resource. For example, `pack builder trust ...` instead of `pack config trust-builder ...`.


Another alternative is that pack config could be a command where the first argument is the resource ala pack config:set <resource>.

That might get tricker b/c some of the config options have context specific usage. Example: setting a run-image-mirror requires and extra arg because you need to know what run-image you are mirroring.

Made my best attempt at trying to prototype what this could look like here. Wasn't able to come up with a favorable format IMO.

hone · 2020-07-15T17:34:53Z

I do agree with the motivations behind this RFC. Probably due to using the Heroku CLI, I have a preference for not using space and it's clearer to me.

text/0000-pack-subcommands.md

ekcasey · 2020-07-16T17:06:05Z

text/0000-pack-subcommands.md

+There would also be some alias subcommands for the `app` set of subcommands for easier usage and compatibility.
+
+* `pack build` -> `pack app build`
+* `pack inspect` -> `pack app inspect`


I like the pack build and pack rebase aliases but I think having a pack inspect that can't inspect builders would be confusing. I instead of making it smarter (which might fall apart if someday we can inspect things that aren't images) I think we should just remove this alias.

Based on some discussions I think we want to move to where pack inspect can auto-detect the image resource type and display the appropriate information. For example, pack inspect <builder> will display builder info, pack inspect <app-image> will display app image information, etc.

text/0000-pack-subcommands.md

ekcasey · 2020-07-16T17:24:13Z

text/0000-pack-subcommands.md

+[alternatives]: #alternatives
+
+- Keep it as it is.
+- Some [Config](#config) subcommands could be more intertwined with their respective resource. For example, `pack builder trust ...` instead of `pack config trust-builder ...`.


That might get tricker b/c some of the config options have context specific usage. Example: setting a run-image-mirror requires and extra arg because you need to know what run-image you are mirroring.

text/0000-pack-subcommands.md

Signed-off-by: Javier Romero <rjavier@vmware.com>

jkutner · 2020-07-21T20:55:16Z

text/0000-pack-subcommands.md

+  completion
+  config
+    default-builder
+      set


so for this one, the command would be pack config default-builder set <value>?

Yes, that's correct.

Can we consider pack config default-builder=<value>?

Is there a proposed similar solution to unsetting? Alternative to pack config default-builder unset.

Maybe, since it's an uncommon operation, it can simply be a flag?

# set resource pack config default-builder=<builder-name> # unset resource pack config default-builder --unset # get pack config default-builder

@jromero is this a change you're hoping to make in the RFC?

I personally prefer pack config default-builder set <value> to pack config default-builder=<builder-name>. Using the verb allow for more natural/consistent extension if we ever want to include other verbs. For example imagine a future where we want additional operations on set (in the other sense) values like pack config trusted-builders add <value>.

If other folks really don't like the verbs I will concede but even in that case I would prefer pack config default-builder <builder-name> to pack config default-builder=<builder-name>. Having an = in a positional argument bugs me for some reason.

@ekcasey I think that's b/c in unix tooling you usually see it on flags and not position args. Take ls for instance:

--format=WORD

Changes in this regard have been made to the RFC. Please re-review.

text/0000-pack-subcommands.md

jromero · 2020-07-28T14:19:37Z

text/0000-pack-subcommands.md

+  completion
+  config
+    default-builder
+      set


Yes, that's correct.

text/0000-pack-subcommands.md

Signed-off-by: Javier Romero <rjavier@vmware.com> Co-authored-by: David Freilich <dfreilich@vmware.com>

text/0000-pack-subcommands.md

Signed-off-by: Javier Romero <rjavier@vmware.com>

dfreilich

Overall, definitely a fan. I think there was a bit of a regression with inspect, so I'd appreciate a clarification there, but 🥳

dfreilich · 2020-09-01T18:28:52Z

text/0000-pack-subcommands.md

+      remove
+      list
+  help
+  inspect


We currently have both pack inspect-image and pack inspect-builder. What would you envision with this inspect command?

dfreilich · 2020-09-01T18:29:06Z

text/0000-pack-subcommands.md

+
+##### Singular
+
+Singular resources would have no subcommands but accept possitional arguments and flags with an implied `get` operation and a common `--unset` flag.


Suggested change

Singular resources would have no subcommands but accept possitional arguments and flags with an implied `get` operation and a common `--unset` flag.

Singular resources would have no subcommands but accept positional arguments and flags with an implied `get` operation and a common `--unset` flag.

dfreilich · 2020-09-01T18:29:36Z

text/0000-pack-subcommands.md

+pack config default-builder --unset
+
+# gets the default builder
+pack config default-builder


If there is none set, what should the default behavior be? Should it run pack suggest-builders?

dfreilich · 2020-09-01T18:31:32Z

text/0000-pack-subcommands.md

+- Only allow manual edits of config file.
+    - A `config` command would allow providing `help` for possible operations and argument values.
+- Some [Config](#config) subcommands could be more intertwined with their respective resource. For example, `pack builder trust ...` instead of `pack config trust-builder ...`.
+    - Other commands are operations on the resource whereas having `config` subcommands would make it more obvious that


I like this point a lot. I think it makes the UX a bit more sensible, though sometimes wordy.

hone · 2020-09-02T19:10:53Z

Moving this to Final Comment Period to close on Sept. 9, 2020.

[#93] Signed-off-by: Ben Hale <bhale@vmware.com>

jromero marked this pull request as ready for review July 8, 2020 20:58

jromero requested a review from a team as a code owner July 8, 2020 20:58

ryanmoran reviewed Jul 8, 2020

View reviewed changes

text/0000-pack-subcommands.md Outdated Show resolved Hide resolved

jromero commented Jul 9, 2020

View reviewed changes

text/0000-pack-subcommands.md Outdated Show resolved Hide resolved

nebhale approved these changes Jul 10, 2020

View reviewed changes

jkutner reviewed Jul 13, 2020

View reviewed changes

text/0000-pack-subcommands.md Outdated Show resolved Hide resolved

jkutner reviewed Jul 13, 2020

View reviewed changes

text/0000-pack-subcommands.md Outdated Show resolved Hide resolved

jkutner reviewed Jul 13, 2020

View reviewed changes

text/0000-pack-subcommands.md Show resolved Hide resolved

hone reviewed Jul 15, 2020

View reviewed changes

text/0000-pack-subcommands.md Show resolved Hide resolved

hone reviewed Jul 15, 2020

View reviewed changes

text/0000-pack-subcommands.md Outdated Show resolved Hide resolved

hone reviewed Jul 15, 2020

View reviewed changes

text/0000-pack-subcommands.md Show resolved Hide resolved

hone reviewed Jul 15, 2020

View reviewed changes

ekcasey approved these changes Jul 16, 2020

View reviewed changes

jromero added 4 commits July 16, 2020 14:41

Create 0000-pack-subcommands.md

ad61f49

Signed-off-by: Javier Romero <rjavier@vmware.com>

Apply suggestions from code review

c25d1f3

Signed-off-by: Javier Romero <rjavier@vmware.com>

Remove app, add prior art, elaborate on config alts

a688d13

Signed-off-by: Javier Romero <rjavier@vmware.com>

Resolve app question

a69fecf

Signed-off-by: Javier Romero <rjavier@vmware.com>

jromero force-pushed the pack-subcommands branch from cfe0da8 to a69fecf Compare July 16, 2020 19:43

Update backwards compat

adce707

Signed-off-by: Javier Romero <rjavier@vmware.com>

jromero mentioned this pull request Jul 17, 2020

Use resource-based subcommands buildpacks/pack#597

Closed

1 task

jkutner reviewed Jul 21, 2020

View reviewed changes

dfreilich reviewed Jul 22, 2020

View reviewed changes

text/0000-pack-subcommands.md Show resolved Hide resolved

yaelharel reviewed Jul 22, 2020

View reviewed changes

text/0000-pack-subcommands.md Outdated Show resolved Hide resolved

jromero commented Jul 28, 2020

View reviewed changes

Apply suggestions from code review

eeb4ade

Signed-off-by: Javier Romero <rjavier@vmware.com> Co-authored-by: David Freilich <dfreilich@vmware.com>

jromero requested review from dfreilich, hone, jkutner and sclevine July 28, 2020 14:22

nebhale requested a review from a team August 11, 2020 22:50

elbandito reviewed Aug 12, 2020

View reviewed changes

text/0000-pack-subcommands.md Show resolved Hide resolved

elbandito reviewed Aug 12, 2020

View reviewed changes

text/0000-pack-subcommands.md Outdated Show resolved Hide resolved

jromero added 4 commits August 26, 2020 14:06

Feedback changes

2c2bc10

Signed-off-by: Javier Romero <rjavier@vmware.com>

Remove unset subcommand

aa2ab79

Signed-off-by: Javier Romero <rjavier@vmware.com>

Update single resource config definition

1758b3f

Signed-off-by: Javier Romero <rjavier@vmware.com>

Remove builder inspect

5c42339

Signed-off-by: Javier Romero <rjavier@vmware.com>

jkutner approved these changes Aug 27, 2020

View reviewed changes

Malax approved these changes Aug 27, 2020

View reviewed changes

Make list resources plural

319c901

Signed-off-by: Javier Romero <rjavier@vmware.com>

jromero requested a review from elbandito September 1, 2020 15:48

dfreilich approved these changes Sep 1, 2020

View reviewed changes

sclevine approved these changes Sep 2, 2020

View reviewed changes

hone added the Final Comment Period label Sep 2, 2020

nebhale added a commit that referenced this pull request Sep 9, 2020

RFC 0058

9d937a7

[#93] Signed-off-by: Ben Hale <bhale@vmware.com>

nebhale merged commit ed1a20d into main Sep 9, 2020

nebhale deleted the pack-subcommands branch September 9, 2020 17:46

ekcasey removed the Final Comment Period label Oct 14, 2020

dfreilich mentioned this pull request Dec 9, 2020

pack config experimental subcommands buildpacks/pack#983

Closed

dfreilich mentioned this pull request Jan 20, 2021

RFC: Create stackify repo #123

Merged

tylerphelan mentioned this pull request Apr 16, 2021

Allow users to configure kp-config with kp buildpacks-community/kpack-cli#160

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pack subcommands #93

pack subcommands #93

jromero commented Jul 8, 2020 •

edited

Loading

hone Jul 15, 2020 •

edited

Loading

ekcasey Jul 16, 2020

jromero Sep 1, 2020

hone commented Jul 15, 2020

ekcasey Jul 16, 2020

jromero Jul 28, 2020

ekcasey Jul 16, 2020

jkutner Jul 21, 2020

jromero Jul 28, 2020

jkutner Jul 30, 2020

jromero Aug 12, 2020

hone Aug 19, 2020

ekcasey Aug 26, 2020

ekcasey Aug 26, 2020

hone Aug 26, 2020

jromero Sep 1, 2020

jromero Jul 28, 2020

dfreilich left a comment

dfreilich Sep 1, 2020

dfreilich Sep 1, 2020

dfreilich Sep 1, 2020

dfreilich Sep 1, 2020

hone commented Sep 2, 2020


		##### Singular

		Singular resources would have no subcommands but accept possitional arguments and flags with an implied `get` operation and a common `--unset` flag.

pack subcommands #93

pack subcommands #93

Conversation

jromero commented Jul 8, 2020 • edited Loading

hone Jul 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hone commented Jul 15, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dfreilich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hone commented Sep 2, 2020

jromero commented Jul 8, 2020 •

edited

Loading

hone Jul 15, 2020 •

edited

Loading