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

fix: change capitalisation of "pod" and "etcd" throughout #336

Closed
wants to merge 4 commits into from

Conversation

craigbox
Copy link
Contributor

POD and ETCD aren't acronyms, so they should never be SHOUTED written in all caps.

This PR is a very considered search and replace across the library. KUBELET_SYSTEM_PODS_ARGS should remain in caps, for example.

Signed-off-by: Craig Box <craigb@armosec.io>
Signed-off-by: Craig Box <craigb@armosec.io>
@matthyx
Copy link
Contributor

matthyx commented Apr 3, 2023

Is it pod or Pod?

@yuleib
Copy link
Contributor

yuleib commented Apr 3, 2023

@YiscahLevySilas1 - do we have any dependencies with other files / libraries which might be affected from this change ? (name convention or any other issue?)

@yuleib yuleib requested a review from YiscahLevySilas1 April 3, 2023 07:15
@craigbox
Copy link
Contributor Author

craigbox commented Apr 3, 2023

Is it pod or Pod?

That is a contentious question.
https://kubernetes.slack.com/archives/C1J0BPD2M/p1679212088970039

SIG Docs guidance is roughly 'When you're talking about a pod, you should say pod; when you're talking about a pod's API object, you should say Pod'.

(We don't have the markdown differentiation.)

craigbox added 2 commits May 30, 2023 14:58
Signed-off-by: Craig Box <craigb@armosec.io>
Signed-off-by: Craig Box <craigb@armosec.io>
@YiscahLevySilas1
Copy link
Collaborator

@craigbox Sorry for the delay here!
In the meantime we've had some changes in the repo and now require that PRs be opened from a branch in this repo and not a fork for tests to pass
@yuleib Perhaps we should block completely the option to open PRs from forks to master if we're not planning to approve them?

@craigbox
Copy link
Contributor Author

In the meantime we've had some changes in the repo and now require that PRs be opened from a branch in this repo and not a fork for tests to pass. @yuleib Perhaps we should block completely the option to open PRs from forks to master if we're not planning to approve them?

Does that not restrict things such that only maintainers can contribute, as only they can create branches in the repo?

@yuleib
Copy link
Contributor

yuleib commented May 31, 2023

@craigbox - We just inserted this option in the past week. The issue is that we cannot perform any test over opened forks because of the secrets which are related to the organization.
i still checking some options to allow forking for outside contributions and running the system tests / rego tests when it arrive to us (for e.g. if we can initiazlize it from our side fork and still see the tests because we are inside the org. ), that way we can allow outside forks (and inside the org. working only with branching )

@rotemamsa
Copy link
Contributor

@yuleib any update on this issue lately?

@yuleib
Copy link
Contributor

yuleib commented Sep 27, 2023

@rotemamsa - same as #323

@craigbox
Copy link
Contributor Author

craigbox commented Oct 2, 2023

replacing with #513.

@craigbox craigbox closed this Oct 2, 2023
@craigbox craigbox deleted the no-shouting branch October 2, 2023 06:33
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.

5 participants