-
Notifications
You must be signed in to change notification settings - Fork 28
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
update byoc doc #116
update byoc doc #116
Conversation
I've been away on Holliday and am just returning. I will still be slow to respond as I get caught up with things, but hope to get this reviewed soon. |
One quick note, please setup commit signing for your contributions. I just realized we have that enforcing, but don't have it documented in the CONTRIBUTING.md file (my apologies for any confusion). I opened #117 to track the addition of that info to the Contributing Docs. More info on how to setup commit signing can be found here: |
30d4120
to
4ea82eb
Compare
Done! Thanks so much! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of small things.
there is one problem I noticed that, in the init code, when the secret exists, we were "Waiting for race winning pod to startup" by checking I tested locally, the init container started successfully. |
FYI: another small issue I ran with is that the black version I installed locally wasn't exactly the same as what we are specifying in the CI job. So previously when I run I noticed from last CI job here https://github.com/tmobile/magtape/runs/4892244656?check_suite_focus=true, there was a specific black version, I changed them locally accordingly and create a new commit to fix the issue 3c8ef3d Maybe we can change the contribution guide to specify the black version we are using in CI job. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
False positive on the codeQL check
/lgtm
What type of PR is this?
/kind documentation
What this PR does / why we need it:
update byoc doc
Which issue(s) this PR fixes:
Fixes #102
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation e.g., usage docs, etc.: