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

Module 14 #18

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Module 14 #18

wants to merge 7 commits into from

Conversation

Iabo77
Copy link
Owner

@Iabo77 Iabo77 commented Jan 4, 2023

Please make sure you are raising this PR against your own repository and not the original. If it says https://github.com/CorndelWithSoftwire in the search bar right now, you are in the wrong place!

Please also don't forget to submit the PR's url to Aptem afterwards (by clicking on the corresponding exercise component, pasting the url in the textbox that appears and then pressing the finish button).

@Iabo77
Copy link
Owner Author

Iabo77 commented Jan 4, 2023

screen capture of loggly logs from minikube running container

image

Copy link

@CorndelDevOps CorndelDevOps 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 for the submission, it looks good.

I left few pedantic comments for you to consider inline.

CLIENT_SECRET: <github_client secret here>
SECRET_KEY: <secretkey here>
CONNECTION_STRING: <connection string here to a mongodb>
#LOGGLY_TOKEN: <loggly token> # optional for external logging to loggly

Choose a reason for hiding this comment

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

nitpick: I don't think this needs to be commented out. It's clear from the inline comment later that this is optional, but leaving it empty should do the trick as well





Choose a reason for hiding this comment

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

Lots of whitespace here

Comment on lines +1 to +2
#unpopulated secrets.yaml file. Populate and rename to secrets.yaml before deploying to minikube

Choose a reason for hiding this comment

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

nitpick: the name could follow the convention similar to .env.template, in which case this would be secrets.yaml.template, making it more obvious that this isn't a yaml file that you should use



# MiniKube Set up
To configure a local minikube deployment of the todo-app. Assuming Docker desktop and Minikube are set up locally.

Choose a reason for hiding this comment

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

Check the phrasing here, because it looks a bit grammatically incorrect

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