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

add kubernetes guestbook example #395

Merged
merged 2 commits into from
Feb 21, 2024
Merged

Conversation

polkx
Copy link
Collaborator

@polkx polkx commented Feb 19, 2024

No description provided.

@polkx polkx requested a review from pawelprazak February 19, 2024 12:29
@polkx polkx self-assigned this Feb 19, 2024
@pawelprazak pawelprazak added the area/examples Usage examples label Feb 19, 2024
@pawelprazak
Copy link
Contributor

For the record, this is the upstream example: https://github.com/pulumi/examples/tree/master/kubernetes-ts-guestbook

@pawelprazak pawelprazak added this to the 0.2.2 milestone Feb 19, 2024
"memory" -> "100Mi"
)
),
ports = List(ContainerPortArgs(containerPort = 6379))
Copy link
Contributor

Choose a reason for hiding this comment

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

the 6379 is used in two contexts and 6 places as a "magic value", I think it would make maintenance easier to create a dedicated variable for this port and reuse it, and as a result make sure any future changes will result in correct and succesfull deployment

Copy link
Contributor

@pawelprazak pawelprazak left a comment

Choose a reason for hiding this comment

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

Thank you! Looks good, found a couple of nits only.

@polkx polkx merged commit 2eb0126 into main Feb 21, 2024
1 check passed
@polkx polkx deleted the kpoliwka/kubernetes-guestbook-example branch February 21, 2024 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/examples Usage examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants