Skip to content

Allow storing multiple commands instead of a single string#900

Merged
natalieparellano merged 9 commits intomainfrom
jab/support-command-array
Sep 22, 2022
Merged

Allow storing multiple commands instead of a single string#900
natalieparellano merged 9 commits intomainfrom
jab/support-command-array

Conversation

@jabrown85
Copy link
Copy Markdown
Contributor

@jabrown85 jabrown85 commented Aug 30, 2022

This is a first step to implementing buildpacks/spec#323. This PR is updating our internal structs to allow for a slice of commands while keeping the external API and behavior the same. A future PR will implement handling multiple commands depending on the API version and changing the behavior of the launcher and metadat file output changes.

Signed-off-by: Jesse Brown jabrown85@gmail.com

@jabrown85 jabrown85 self-assigned this Aug 30, 2022
@jabrown85 jabrown85 requested a review from a team as a code owner August 30, 2022 18:35
Copy link
Copy Markdown
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.

@jabrown85 apologies for the slow review! I added a couple comments but overall this LGTM

@jabrown85
Copy link
Copy Markdown
Contributor Author

@natalieparellano did you want to take another look or 👍 to merge?

@natalieparellano
Copy link
Copy Markdown
Member

@jabrown85 apologies for the slow reply! I made two suggestions for the platform api boundary (this will go out in 0.10, not 0.11) but otherwise this looks good to me!

jabrown85 and others added 5 commits September 21, 2022 12:36
This is a first step to implementing #322. This PR is updating our internal structs to allow for a slice of commands while keeping the external API and behavior the same. A future PR will implement handling multiple commands depending on the API version and changing the behavior of the launcher and metadat file output changes.

Signed-off-by: Jesse Brown <jabrown85@gmail.com>
Signed-off-by: Jesse Brown <jabrown85@gmail.com>
Signed-off-by: Jesse Brown <jabrown85@gmail.com>
Signed-off-by: Jesse Brown <jabrown85@gmail.com>
Signed-off-by: Jesse Brown <jabrown85@gmail.com>

Co-authored-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Jesse Brown <jabrown85@gmail.com>
@jabrown85 jabrown85 force-pushed the jab/support-command-array branch from e893f97 to 4851341 Compare September 21, 2022 17:43
@jabrown85
Copy link
Copy Markdown
Contributor Author

I need to iterate on this some more after the version changes

jabrown85 and others added 3 commits September 21, 2022 13:43
Signed-off-by: Natalie Arellano <narellano@vmware.com>
Fix editing daemon settings

Signed-off-by: Jesse Brown <jabrown85@gmail.com>
@natalieparellano natalieparellano merged commit 5fa84a4 into main Sep 22, 2022
@natalieparellano natalieparellano deleted the jab/support-command-array branch September 22, 2022 19:40
jabrown85 added a commit that referenced this pull request Sep 22, 2022
Continuation of work in #900. For platform >= 0.10, lifecycle will write command as an array to metadata.toml and the resulting image label. No other intended behavior changes at this time. Previous APIs will continue to write as a string.

Signed-off-by: Jesse Brown <jabrown85@gmail.com>
natalieparellano pushed a commit that referenced this pull request Sep 29, 2022
natalieparellano pushed a commit that referenced this pull request Sep 29, 2022
)"

Signed-off-by: Natalie Arellano <narellano@vmware.com>
natalieparellano pushed a commit that referenced this pull request Sep 29, 2022
)" (#919)

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Signed-off-by: Natalie Arellano <narellano@vmware.com>
jabrown85 added a commit that referenced this pull request Sep 29, 2022
* Allow storing multiple commands instead of a single string

This is a first step to implementing #322. This PR is updating our internal structs to allow for a slice of commands while keeping the external API and behavior the same. A future PR will implement handling multiple commands depending on the API version and changing the behavior of the launcher and metadat file output changes.

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* Comment on usage of cmp.Option

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* Add comment on Matches interface usage for testing

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* Added test for launch.toml decoding branching logic

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* Apply suggestions from code review

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

Co-authored-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* fixup! Apply suggestions from code review

* fixup! Apply suggestions from code review

* Fix editing daemon settings

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Signed-off-by: Jesse Brown <jabrown85@gmail.com>
Signed-off-by: Natalie Arellano <narellano@vmware.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com>
natalieparellano pushed a commit that referenced this pull request Sep 30, 2022
* Allow storing multiple commands instead of a single string (#900)

* Allow storing multiple commands instead of a single string

This is a first step to implementing #322. This PR is updating our internal structs to allow for a slice of commands while keeping the external API and behavior the same. A future PR will implement handling multiple commands depending on the API version and changing the behavior of the launcher and metadat file output changes.

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* Comment on usage of cmp.Option

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* Add comment on Matches interface usage for testing

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* Added test for launch.toml decoding branching logic

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* Apply suggestions from code review

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

Co-authored-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* fixup! Apply suggestions from code review

* fixup! Apply suggestions from code review

* Fix editing daemon settings

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Signed-off-by: Jesse Brown <jabrown85@gmail.com>
Signed-off-by: Natalie Arellano <narellano@vmware.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com>

* WIP

Signed-off-by: Natalie Arellano <narellano@vmware.com>

* Fixing up existing tests

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* Remove now unused process specific decode paths

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* fixup! Remove now unused process specific decode paths

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* fixup! Remove now unused process specific decode paths

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* fixup! Remove now unused process specific decode paths

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* fixup! Remove now unused process specific decode paths

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* fixup! Remove now unused process specific decode paths

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* Clean up more serialization paths

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* Remove toml wrapping code

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* Added comment on UnmarshalTOML

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* Put back code I didn't mean to remove

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* Update launch/launch.go

Co-authored-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Jesse Brown <jabrown85@gmail.com>

* removed line

Signed-off-by: Jesse Brown <jabrown85@gmail.com>

Signed-off-by: Jesse Brown <jabrown85@gmail.com>
Signed-off-by: Natalie Arellano <narellano@vmware.com>
Co-authored-by: Natalie Arellano <narellano@vmware.com>
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