search

brigadecore / brigade

Event-driven scripting for Kubernetes
https://brigade.sh/
Apache License 2.0
2.4k stars 247 forks source link

Update deploy.md #1888

Closed dtaskai closed 2 years ago

dtaskai commented 2 years ago

Updated according to issue #1882, fixed typo

krancour commented 2 years ago

@dtaskai thanks for the contribution. I agree with these changes, but I don't think they resolve #1882. That issue is about the contradiction that specifying a MongoDB password was listed as both required and optional.

Do you want to try to fix that as part of this PR? Or do you want me to merge this typo fix and you can optionally open a second PR that resolves #1882 (if that interests you)?

dtaskai commented 2 years ago

@krancour I might be misinterpreting the issue, since the password field was listed both as required and optional, I have removed the part that listed it as required and moved the rest of the instructions to the optional part, may I ask for further clarification if I'm getting the issue wrong? I'll be glad to help!

netlify[bot] commented 2 years ago

Deploy Preview for brigade-docs ready!

Name Link
Latest commit 8905c4020b95011829a91b25e94bcf70e62769fd
Latest deploy log https://app.netlify.com/sites/brigade-docs/deploys/623b62a0fab4500008a2b28c
Deploy Preview https://deploy-preview-1888--brigade-docs.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

krancour commented 2 years ago

@dtaskai I see you updated the commit. This LGTM now. Thanks!

Your commit doesn't appear to be verified though (cryptographically signed). Any chance of fixing that?

More info here: https://github.com/brigadecore/community/blob/main/contributing.md#gpg-signature

dtaskai commented 2 years ago

@krancour Followed the steps, should be fine now, LMK if it's not right.

krancour commented 2 years ago

@dtaskai looks perfect. Thanks!

krancour commented 2 years ago

@dtaskai I've been looking into #1882 more and it turns out this isn't the proper resolution.

The original text was almost correct and the only issue is that the field name mongodb.auth.passwords in the bulleted list was incorrect. It should have been mongodb.auth.rootPassword.

If just that one thing is changed, everything else was correct.

dtaskai commented 2 years ago

@krancour After reading the Bitnami Docker image documentation it is clear to me now too, reversed the changes and applied your fix!

krancour commented 2 years ago

@dtaskai perfect. Thank you for your patience in sorting this out.