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

Refactors #2635

Merged
merged 16 commits into from
Sep 28, 2023
Merged

Refactors #2635

merged 16 commits into from
Sep 28, 2023

Conversation

T4rk1n
Copy link
Contributor

@T4rk1n T4rk1n commented Aug 30, 2023

  • Pick props instead of keys for dcc.Input, dcc.Textarea, dcc.Slider, dcc.RangeSlider
  • Batch redux dispatches in APIController.

@T4rk1n T4rk1n requested a review from alexcjohnson as a code owner August 30, 2023 20:28
@T4rk1n T4rk1n force-pushed the optim/refact branch 2 times, most recently from dd159eb to 1f82f23 Compare September 27, 2023 14:31
alexcjohnson
alexcjohnson reviewed Sep 27, 2023
View reviewed changes
.circleci/config.yml Outdated
@@ -3,7 +3,7 @@ version: 2.1
orbs:
win: circleci/windows@2.4.0
percy: percy/agent@0.1.3
browser-tools: circleci/browser-tools@1.4.3
browser-tools: circleci/browser-tools@1.4.5
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a 1.4.6 now, but we can wait on that if this one works.

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh maybe that would let us remove the chrome 116 pin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I think that was fixed and there is also now a proper chromedriver version I think that was the cause for dvpc001 failures.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I unpinned the chrome version and the dvpc was failing again.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK - please make an issue to come back to this

alexcjohnson
alexcjohnson reviewed Sep 27, 2023
View reviewed changes
tests/integration/devtools/test_props_check.py Outdated
@@ -216,3 +218,5 @@ def display_content(pathname):
assert dash_duo.get_logs(), tc
else:
assert dash_duo.get_logs() == [], tc

time.sleep(0.1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

what effect does this sleep have, coming after all the assertions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test was failing but I am not sure that it needs this.

alexcjohnson
alexcjohnson approved these changes Sep 27, 2023
View reviewed changes
Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃 LGTM. If you don't want to investigate browser-tools@1.4.6 now, please make an issue to come back and remove the chrome pin later.

@T4rk1n T4rk1n force-pushed the optim/refact branch 2 times, most recently from 65397b4 to 8ae9488 Compare September 28, 2023 14:24
@T4rk1n T4rk1n merged commit d9b8d17 into dev Sep 28, 2023
@T4rk1n T4rk1n deleted the optim/refact branch September 28, 2023 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexcjohnson alexcjohnson alexcjohnson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@T4rk1n @alexcjohnson