Bug 1787210: deployment config page contains an error in the pod counter #4157
Conversation
|
@dtaylor113: This pull request references Bugzilla bug 1787210, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
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-robot
added
bugzilla/valid-bug
There was a problem hiding this comment.
/assign @christianvogt
Christian, can you take a look since you're more familiar with this component?
| @@ -1,3 +1,4 @@ | |||
| /* eslint-disable react-hooks/exhaustive-deps */ | |||
There was a problem hiding this comment.
We should fix the problem instead of ignoring. We definitely shouldn't ignore at the file level.
There was a problem hiding this comment.
It wants me to add clickCount to the dependency array which I don't think is necessary, I don't want to execute useEffect when clickCount changes, just obj.spec.replicas. I can move the ignore to line-level
There was a problem hiding this comment.
fixed, moved ignore to line-level.
There was a problem hiding this comment.
@dtaylor113 whenever we disable rules, sometimes it's obvious, but when it's not please add a comment otherwise someone else will come in and fix the lint error.
There was a problem hiding this comment.
Hi @christianvogt, added eslint comment. -thanks
dtaylor113
force-pushed
the
deployment-details-error-in-pod-counter
branch
from
7c4e5da
to
1532107
Compare
openshift-ci-robot
added
size/S
|
This component has other issues. The intent is that we should be able to continuously click the buttons and adjust the value even though the system hasn't yet caught up to the set values. I tested this change and it certainly did fix the issue. I think the fix is sufficient and further refinements could be tackled in a tech debt item which I can log. |
…ter [openshift-4.4]
dtaylor113
force-pushed
the
deployment-details-error-in-pod-counter
branch
from
1532107
to
094d7ec
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: christianvogt, dtaylor113 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 |
openshift-ci-robot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@dtaylor113: All pull requests linked via external trackers have merged. Bugzilla bug 1787210 has been moved to the MODIFIED state. In response to this:
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. |
Added
useEffectto update state variable if # pods changed outside of PodRing component. This happens in the Edit Count modal, which closes without reloading page or remounting PodRing component.