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.

Sign up for GitHub

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

Jump to bottom

feat: move supported resources from hard-coded to ConfigMap #196

Merged
merged 1 commit into from
Sep 2, 2024
Merged

feat: move supported resources from hard-coded to ConfigMap #196

merged 1 commit into from
Sep 2, 2024

Conversation

MaromC
Copy link
Contributor

@MaromC MaromC commented Aug 20, 2024

No description provided.

@dana-prow-ci dana-prow-ci bot requested a review from mzeevi August 20, 2024 09:05
@MaromC MaromC force-pushed the hardCodedToConfig branch 5 times, most recently from ee50c48 to 0c5ef7a Compare August 26, 2024 14:14
@MaromC MaromC force-pushed the hardCodedToConfig branch from 0c5ef7a to 3c2e120 Compare August 28, 2024 14:49
@mzeevi
Copy link
Contributor

mzeevi commented Aug 29, 2024

Please rebase now that #197 has been merged and amend your code to look like that

@MaromC MaromC force-pushed the hardCodedToConfig branch 8 times, most recently from b366275 to 387952b Compare September 1, 2024 08:50
mzeevi
mzeevi requested changes Sep 1, 2024
View reviewed changes
.uuid Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
config/manager/kustomization.yaml Outdated Show resolved Hide resolved
config/rbac/configmap_role.yaml Outdated Show resolved Hide resolved
config/rbac/configmap_role_binding.yaml Outdated Show resolved Hide resolved
mh.yaml Outdated Show resolved Hide resolved
ns.yaml Outdated Show resolved Hide resolved
rootns.yaml Outdated Show resolved Hide resolved
sns.yaml Outdated Show resolved Hide resolved
uq.yaml Outdated Show resolved Hide resolved
@MaromC MaromC force-pushed the hardCodedToConfig branch 6 times, most recently from 6b4d130 to e6e2c0b Compare September 1, 2024 14:58
mzeevi
mzeevi requested changes Sep 1, 2024
View reviewed changes
Copy link
Contributor

@mzeevi mzeevi left a comment

Choose a reason for hiding this comment

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

Added new comments + you left some comments unaddressed. Also, amend the commit message, and delete unneeded files

internal/quota/default.go Outdated Show resolved Hide resolved
internal/quota/default.go Outdated Show resolved Hide resolved
internal/quota/default.go Outdated Show resolved Hide resolved
internal/quota/default.go Outdated Show resolved Hide resolved
internal/quota/snsquota.go Outdated Show resolved Hide resolved
internal/subnamespace/controller.go Outdated Show resolved Hide resolved
internal/subnamespace/init.go Outdated Show resolved Hide resolved
internal/subnamespace/sync.go Outdated Show resolved Hide resolved
internal/subnamespace/sync.go Outdated Show resolved Hide resolved
internal/updatequota/controller.go Outdated Show resolved Hide resolved
mzeevi
mzeevi requested changes Sep 2, 2024
View reviewed changes
Copy link
Contributor

@mzeevi mzeevi left a comment

Choose a reason for hiding this comment

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

Some minor changes, also please use a signed commit and make sure it's your own user which authors and commits. Currently it shows as this for some reason:

dvirgilad authored and maromcohen committed

internal/subnamespace/commonvalidator.go Outdated Show resolved Hide resolved
internal/subnamespace/init.go Outdated Show resolved Hide resolved
internal/subnamespace/sync.go Outdated Show resolved Hide resolved
internal/subnamespace/sync.go Outdated Show resolved Hide resolved
mzeevi
mzeevi requested changes Sep 2, 2024
View reviewed changes
internal/subnamespace/sync.go Outdated Show resolved Hide resolved
@MaromC MaromC force-pushed the hardCodedToConfig branch 2 times, most recently from c3dcf5f to 7539400 Compare September 2, 2024 12:42
feat: move supported resources from hard-coded to ConfigMap
b909f41 
Signed-off-by: maromcohen <marom@dana.org>
@mzeevi mzeevi changed the title feat: Move supported resources from hard-coded to ConfigMap feat: move supported resources from hard-coded to ConfigMap Sep 2, 2024
mzeevi
mzeevi approved these changes Sep 2, 2024
View reviewed changes
Copy link
Contributor

@mzeevi mzeevi left a comment

Choose a reason for hiding this comment

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

Nice work!

/lgtm
/approve

@dana-prow-ci dana-prow-ci bot added the lgtm label Sep 2, 2024
@dana-prow-ci dana-prow-ci
Copy link
Contributor

dana-prow-ci bot commented Sep 2, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MaromC, mzeevi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@dana-prow-ci dana-prow-ci bot added the approved label Sep 2, 2024
@dana-prow-ci dana-prow-ci bot merged commit 3e2fc25 into main Sep 2, 2024
3 checks passed
@mzeevi mzeevi deleted the hardCodedToConfig branch September 4, 2024 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mzeevi mzeevi mzeevi approved these changes

Assignees

@mzeevi mzeevi

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MaromC @mzeevi