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

Yj dartdevembedder 2488 #2531

Merged
merged 12 commits into from
Dec 3, 2024
Merged

Yj dartdevembedder 2488 #2531

merged 12 commits into from
Dec 3, 2024

Conversation

jyameo
Copy link
Contributor

@jyameo jyameo commented Nov 29, 2024 •
edited
Loading

  • Added DartRuntimeDebugger class to streamline JS expression generation and added support for
    the dartDevEmbedder API for methods: getObjectMetadata, getObjectFieldNames, getSubRange, and getFunctionMetadata - #2488
  • Reenabled tests in frontend_server_ddc_and_canary_evaluate_test.dart. - #2488

@jyameo jyameo requested a review from srujzs November 29, 2024 22:42
srujzs
srujzs requested changes Dec 2, 2024
View reviewed changes
Copy link
Contributor

@srujzs srujzs left a comment

Choose a reason for hiding this comment

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

Thanks Jessy! Looks like that test is passing, nice work!

We should look into getting some other tests working with the library bundle format so we can test other debugger APIs but I'm guessing you're already aware of that. :)

dwds/CHANGELOG.md Outdated Show resolved Hide resolved
dwds/lib/src/debugging/inspector.dart Outdated Show resolved Hide resolved
dwds/lib/src/loaders/load_strategy_handler.dart Outdated Show resolved Hide resolved
dwds/lib/src/loaders/load_strategy_handler.dart Outdated Show resolved Hide resolved
dwds/lib/src/loaders/load_strategy_handler.dart Outdated Show resolved Hide resolved
jyameo
jyameo commented Dec 2, 2024
View reviewed changes
Copy link
Contributor Author

@jyameo jyameo left a comment

Choose a reason for hiding this comment

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

@srujzs I have the changes left in the PR. Please let me know if more changes are needed

@jyameo jyameo requested a review from srujzs December 2, 2024 22:22
srujzs
srujzs reviewed Dec 2, 2024
View reviewed changes
dwds/lib/src/loaders/dart_runtime_debugger.dart Outdated Show resolved Hide resolved
dwds/lib/src/loaders/ddc.dart Outdated Show resolved Hide resolved
dwds/lib/src/loaders/ddc.dart Outdated Show resolved Hide resolved
dwds/lib/src/loaders/dart_runtime_debugger.dart Outdated Show resolved Hide resolved
@jyameo jyameo requested a review from srujzs December 3, 2024 16:41
srujzs
srujzs approved these changes Dec 3, 2024
View reviewed changes
Copy link
Contributor

@srujzs srujzs left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM % my final comment. Thanks for the edits!

dwds/lib/src/loaders/ddc.dart Outdated Show resolved Hide resolved
@jyameo
Copy link
Contributor Author

jyameo commented Dec 3, 2024

@srujzs what do you think about renaming the test frontend_server_ddc_and_canary_evaluate_test to frontend_server_library_bundle_evaluate_test?

@jyameo jyameo requested a review from srujzs December 3, 2024 18:27
@srujzs
Copy link
Contributor

srujzs commented Dec 3, 2024

what do you think about renaming the test frontend_server_ddc_and_canary_evaluate_test to frontend_server_library_bundle_evaluate_test?

Talked offline but, frontend_server_ddc_library_bundle_evaluate_test SGTM.

@jyameo jyameo merged commit 564bd1b into dart-lang:main Dec 3, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srujzs srujzs srujzs approved these changes

Assignees
No one assigned
Labels
package:dwds
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jyameo @srujzs