[Freestyler] Show a confirmation UI before running a code with side effects [devtools/devtools-frontend : main]
0 views
Skip to first unread message
Ergün Erdoğmuş (Gerrit)
Jul 2, 2024, 11:18:32 AM (18 hours ago) Jul 2
to Alex Rudenko, devtools-rev...@chromium.org
Attention needed from Alex Rudenko
Ergün Erdoğmuş voted![]( "Open in Gerrit")
| Auto-Submit | +1 |
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Alex Rudenko
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
2:07 AM (3 hours ago) 2:07 AM
to Ergün Erdoğmuş, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org
Attention needed from Ergün Erdoğmuş
Alex Rudenko added 1 comment![]( "Open in Gerrit")
File front_end/panels/freestyler/FreestylerAgent.ts
Line 238, Patchset 2 (Latest):
const shouldAllowSideEffect = await this.#confirmSideEffect(action);Alex Rudenko . unresolved
I think we should probably use yield to produce a SideEffect step and then replace the async generator loop with iterator and return true/false when resume the iterator. This implementation is prone to infinite timeouts.
Related details
Attention is currently required from:
- Ergün Erdoğmuş
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
2:26 AM (3 hours ago) 2:26 AM
to Ergün Erdoğmuş, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org
Attention needed from Ergün Erdoğmuş
Alex Rudenko added 1 comment![]( "Open in Gerrit")
File front_end/panels/freestyler/FreestylerAgent.ts
Line 238, Patchset 2 (Latest):
const shouldAllowSideEffect = await this.#confirmSideEffect(action);Alex Rudenko . resolved
I think we should probably use yield to produce a SideEffect step and then replace the async generator loop with iterator and return true/false when resume the iterator. This implementation is prone to infinite timeouts.
Alex Rudenko
we can try that later.
Related details
Attention is currently required from:
- Ergün Erdoğmuş
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
2:26 AM (3 hours ago) 2:26 AM
to Ergün Erdoğmuş, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org
Attention needed from Ergün Erdoğmuş
Alex Rudenko voted![]( "Open in Gerrit")
| Code-Review | +1 |
| Commit-Queue | +2 |
Related details
Attention is currently required from:
- Ergün Erdoğmuş
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Devtools-frontend LUCI CQ (Gerrit)
3:12 AM (2 hours ago) 3:12 AM
to Ergün Erdoğmuş, Alex Rudenko, devtools-rev...@chromium.org
Devtools-frontend LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[Freestyler] Show a confirmation UI before running a code with side effects
I didn't special case `Fix this issue` case as I didn't implement that case yet in the UI. I'll tackle it as part of b/338931042.
See the bug for the screenshot.
Fixed: 347677554
Change-Id: Ibe73f23c38636d9deb85f145387b8bfdf3259741
Auto-Submit: Ergün Erdoğmuş <erg...@chromium.org>
Reviewed-by: Alex Rudenko <alexr...@chromium.org>
Commit-Queue: Alex Rudenko <alexr...@chromium.org>
Files:
- M front_end/panels/freestyler/FreestylerAgent.test.ts
- M front_end/panels/freestyler/FreestylerAgent.ts
- M front_end/panels/freestyler/FreestylerEvaluateAction.test.ts
- M front_end/panels/freestyler/FreestylerEvaluateAction.ts
- M front_end/panels/freestyler/FreestylerPanel.test.ts
- M front_end/panels/freestyler/FreestylerPanel.ts
- M front_end/panels/freestyler/components/FreestylerChatUi.ts
- M front_end/panels/freestyler/components/freestylerChatUi.css
Change size: L
Delta: 8 files changed, 291 insertions(+), 14 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Alex Rudenko
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages