Skip to content
This repository was archived by the owner on Mar 3, 2025. It is now read-only.

(chore): manifests cleanup#24

Merged
anik120 merged 2 commits intooperator-framework:mainfrom
everettraven:fix/kustomize-stuff
Apr 10, 2023
Merged

(chore): manifests cleanup#24
anik120 merged 2 commits intooperator-framework:mainfrom
everettraven:fix/kustomize-stuff

Conversation

@everettraven
Copy link
Copy Markdown
Collaborator

@everettraven everettraven commented Apr 5, 2023

Description

Motivation
Some post-merge comments here: #16 (review)

Copy link
Copy Markdown
Member

@anik120 anik120 left a comment

Choose a reason for hiding this comment

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

The manifest cleanup part looks good to me. Deploy target is also being fixed in #26 and it pays it a little more indepth attention to it, so we can probably leave the make deploy target cleanup part of this PR out and just leave the manifest cleanup part

Also would be nice to have a line that says
"Follow up for "
in the commit (and the first comment of the PR) so that there's a proper paper trail.

@everettraven everettraven force-pushed the fix/kustomize-stuff branch from e302b07 to 09a572a Compare April 7, 2023 19:39
@everettraven
Copy link
Copy Markdown
Collaborator Author

@anik120 updated

Comment thread config/manager/manager.yaml Outdated
Comment on lines 41 to 45
# TODO(user): For common cases that do not require escalating privileges
# it is recommended to ensure that all your Pods/Containers are restrictive.
# More info: https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted
# Please uncomment the following code if your project does NOT have to work on old Kubernetes
# versions < 1.19 or on vendors versions which do NOT support this field by default (i.e. Openshift < 4.11 ).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this todo comments feels redundant at this point

Copy link
Copy Markdown
Collaborator Author

@everettraven everettraven Apr 10, 2023

Choose a reason for hiding this comment

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

Cleaned up all the todos I could find and removed the old crds in 1657cb0

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 10, 2023
Signed-off-by: Bryce Palmer <bpalmer@redhat.com>
Signed-off-by: Bryce Palmer <bpalmer@redhat.com>
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 10, 2023
@everettraven everettraven changed the title (fix): make deploy target and manifest cleanup (chore): manifests cleanup Apr 10, 2023
Copy link
Copy Markdown
Member

@anik120 anik120 left a comment

Choose a reason for hiding this comment

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

lgtm

@anik120 anik120 merged commit 9e06b2b into operator-framework:main Apr 10, 2023
anik120 added a commit to anik120/catalogd that referenced this pull request Apr 11, 2023
anik120 added a commit that referenced this pull request Apr 12, 2023
* run make generate as follow up to #24

* (actions) add sanity check

* (Makefile) fix target tidy

Tidy was being instructed to `go mod tidy` the hack/tools folder,
which was being done when the tools were being downloaded using
go modules (the main branch has the setup to download the tools
using go installer instead)

* (actions) use caching for go cmds
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants