-
Notifications
You must be signed in to change notification settings - Fork 5
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.
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
Support editing lists, args, and kwargs #512
Conversation
720b8aa
to
d16611e
Compare
8bb2172
to
5c3d68a
Compare
5c3d68a
to
c26a29e
Compare
pipelineIndex: number, | ||
update: Partial<PipelineDefinition["model"]> | ||
) => | ||
setPartialConfig({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be safer to use
setPartialConfig(partialConfig => ({...partialConfig, etc...}))
https://react.dev/reference/react/useState#updating-state-based-on-the-previous-state
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh cool! I didn't know that existed. 👍
It is like that in more than 20 other places, so I'll make another refactor PR to cover everything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you decide to do the changes in this file in this PR there are a few more instances in this file as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, yeah, I am just talking about this file: 21 instances of setPartialConfig({ ...partialConfig
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with comments
Description:
Checklist:
You should check all boxes before the PR is ready. If a box does not apply, check it to acknowledge it.
ran
pre-commit run --all-files
at the end.our users.
README
files and our wiki for any big design decisions, if relevant.