Skip to content

Update documentation for initContainers addition in spring-cloud-deployer #5866

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

Closed

Conversation

corneil
Copy link
Contributor

@corneil corneil commented Jul 9, 2024

@corneil corneil requested a review from cppwfs July 9, 2024 14:23
@corneil corneil added this to the 2.11.5 milestone Jul 9, 2024
@corneil corneil changed the title [DO NOT MERGE] Update documentation for initContainers addition in spring-cloud-deployer Update documentation for initContainers addition in spring-cloud-deployer Aug 5, 2024
@@ -1134,6 +1134,11 @@ The following example shows how you can configure an Init Container for an appli
[source,options=nowrap]
----
deployer.<application>.kubernetes.initContainer={containerName: 'test', imageName: 'busybox:latest', commands: ['sh', '-c', 'echo hello']}
# alternative for multiple init containers
deployer.<application>.kubernetes.initContainers=[{containerName:'test',imageName:'busybox:latest',commands:['sh','-c','echo hello']},{containerName:'test2',imageName:'busybox:latest',commands:['sh','-c','echo world']}]
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick : Make sure we have a space after the comma so samples line up with each other.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should the alternative example go below the multiple containers example?

Copy link
Contributor

Choose a reason for hiding this comment

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

inidividually is misspelled ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -1134,6 +1134,11 @@ The following example shows how you can configure an Init Container for an appli
[source,options=nowrap]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should line 1131 be updated to say "The following example shows how you can configure an Init Container or containers for an application:"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@corneil corneil force-pushed the main branch 2 times, most recently from c0462f2 to f0fb797 Compare August 8, 2024 11:55
Copy link
Contributor

@cppwfs cppwfs left a comment

Choose a reason for hiding this comment

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

I'll fix the , issue when I merge.

@cppwfs cppwfs added the type/forwardport Is a issue to track forward, use with branch/xxx label Aug 23, 2024
@cppwfs
Copy link
Contributor

cppwfs commented Aug 23, 2024

LGTM Squashed Merged

@cppwfs
Copy link
Contributor

cppwfs commented Aug 23, 2024

forward ported to main-3

@cppwfs cppwfs closed this Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/forwardport Is a issue to track forward, use with branch/xxx
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants