Skip to content

Conversation

@ncaak
Copy link
Contributor

@ncaak ncaak commented May 25, 2023

summary

  • Added a new ConfigMapWatcher that will provide with configuration and updates
  • Added basic logic to use with current Validation engine

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented May 25, 2023

@ncaak: This pull request references DVO-114 which is a valid jira issue.

In response to this:

summary

  • Added a new ConfigMapWatcher that will provide with configuration and updates
  • Added basic logic to use with current Validation engine

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci
Copy link

openshift-ci bot commented May 25, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci
Copy link

openshift-ci bot commented May 25, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ncaak

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

@ncaak
Copy link
Contributor Author

ncaak commented May 25, 2023

/test lint

@ncaak ncaak marked this pull request as ready for review May 25, 2023 08:22
@openshift-ci openshift-ci bot requested review from jmelis and maorfr May 25, 2023 08:22
@codecov-commenter
Copy link

codecov-commenter commented May 25, 2023

Codecov Report

Merging #251 (c09a314) into master (9a87b22) will decrease coverage by 1.00%.
The diff coverage is 13.33%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #251      +/-   ##
==========================================
- Coverage   44.39%   43.40%   -1.00%     
==========================================
  Files          22       23       +1     
  Lines         910      940      +30     
==========================================
+ Hits          404      408       +4     
- Misses        472      497      +25     
- Partials       34       35       +1     
Impacted Files Coverage Δ
pkg/controller/configmap_watcher.go 13.33% <13.33%> (ø)

@tremes
Copy link
Contributor

tremes commented May 25, 2023

I think I like the approach in 34af45a a little bit more, because it allows to define the informer only for the particular namespace.

@ncaak
Copy link
Contributor Author

ncaak commented May 25, 2023

I think I like the approach in 34af45a a little bit more, because it allows to define the informer only for the particular namespace.

Ok, I'll rework it using factory pattern. Maybe that way also we can add more unit tests.

@ncaak
Copy link
Contributor Author

ncaak commented May 25, 2023

/hold

@tremes
Copy link
Contributor

tremes commented Jun 1, 2023

OK so let's merge this and we can continue to build upon it. Thanks.
/lgtm

@openshift-ci openshift-ci bot added the lgtm label Jun 1, 2023
@openshift-merge-robot openshift-merge-robot merged commit d0931ef into app-sre:master Jun 1, 2023
@ncaak ncaak deleted the DVO-114/configmap-controller branch February 7, 2025 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants