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

fix: display Command remaining parts if Args is nil #1977

Merged
merged 6 commits into from
Jan 18, 2024

Conversation

kotapeter
Copy link
Contributor

@kotapeter kotapeter commented Nov 15, 2023

Summary

Since lifecycle@0.15.0 the Command is an array that can contain args too. We just pick the first element of Command.Entries so the remaining parts are gone.

Output

Before

Screenshot 2023-11-15 at 08 30 13

After

Screenshot 2023-11-15 at 08 30 07

Documentation

  • Should this change be documented?
    • Yes, see #___
    • No

Related

Resolves #1976

@kotapeter kotapeter requested review from a team as code owners November 15, 2023 07:30
@github-actions github-actions bot added the type/enhancement Issue that requests a new feature or improvement. label Nov 15, 2023
@github-actions github-actions bot added this to the 0.33.0 milestone Nov 15, 2023
Signed-off-by: Peter Kota <kotapeter@gmail.com>
Signed-off-by: Peter Kota <kotapeter@gmail.com>
@kotapeter kotapeter force-pushed the fix-args-display-inspect branch from f610c78 to 0127857 Compare November 15, 2023 07:40
@natalieparellano
Copy link
Member

natalieparellano commented Nov 15, 2023

@kotapeter thanks for this. This was an unfortunate oversight in the implementation of these APIs.

I wonder if we want to display both the "always args" (args that are always provided, regardless of user input - these are proc.Command.Entries[1:] if they exist) and the "overridable args" (these are proc.Args). Of course, older APIs (where proc.Args are "always args") would need to be handled differently so as to be displayed correctly, which might bloat this PR. I wonder if in the short-term we can just concatenate them.

@jjbustamante WDYT?

@natalieparellano
Copy link
Member

Cross-posting from the issue: #1976 (comment)

@jjbustamante jjbustamante merged commit 5dffab6 into buildpacks:main Jan 18, 2024
18 checks passed
@kotapeter kotapeter deleted the fix-args-display-inspect branch February 3, 2024 17:59
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.

pack inspect does not provide ARGS for npm project
3 participants