Skip to content

devtools: Replace new with register for TargetConfigurationActor#43915

Merged
mukilan merged 1 commit into
servo:mainfrom
sabbCodes:issue-43800-target-configuration-actor
Apr 6, 2026
Merged

devtools: Replace new with register for TargetConfigurationActor#43915
mukilan merged 1 commit into
servo:mainfrom
sabbCodes:issue-43800-target-configuration-actor

Conversation

@sabbCodes

Copy link
Copy Markdown
Contributor

Replaced new with register for TargetConfigurationActor in target_configuration.rs & watcher.rs

Testing: No testing required, compiles successfully.
Fixes: Part of #43800

@sabbCodes sabbCodes requested review from atbrakhi and eerii as code owners April 3, 2026 21:45
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 3, 2026
@sabbCodes sabbCodes force-pushed the issue-43800-target-configuration-actor branch from 3299c8f to cdf5e5d Compare April 4, 2026 12:40
Comment thread components/devtools/actors/watcher.rs Outdated
let network_parent_name = NetworkParentActor::register(registry);
let target_configuration =
TargetConfigurationActor::new(registry.new_name::<TargetConfigurationActor>());
let target_configuration = TargetConfigurationActor::register(registry);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
let target_configuration = TargetConfigurationActor::register(registry);
let target_configuration_name = TargetConfigurationActor::register(registry);

Comment thread components/devtools/actors/watcher.rs Outdated
browsing_context_name,
network_parent_name,
target_configuration: target_configuration.name(),
target_configuration,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The field name should be updated here.

Suggested change
target_configuration,
target_configuration_name,

@servo-highfive servo-highfive added S-needs-rebase There are merge conflict errors. S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 5, 2026
@sabbCodes sabbCodes force-pushed the issue-43800-target-configuration-actor branch from cdf5e5d to 7845f8c Compare April 5, 2026 21:38
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. S-needs-rebase There are merge conflict errors. labels Apr 5, 2026
@sabbCodes sabbCodes force-pushed the issue-43800-target-configuration-actor branch from 7845f8c to 98adea4 Compare April 5, 2026 21:49
Comment thread components/devtools/actors/watcher.rs Outdated
browsing_context_name,
network_parent_name,
target_configuration: target_configuration.name(),
target_configuration: target_configuration_name,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
target_configuration: target_configuration_name,
target_configuration_name,

My previous comment about target_configuration field still applies. We'll need to rename this field in WatcherActor. But the current change is modifying WatcherActor::new which will be be replaced by WatcherActor::register. I'd recommend that you wait until #43911 lands to avoid conflicts and to ensure you are modifying the WatcherActor::register method.

@servo-highfive servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 6, 2026
Signed-off-by: Sabb <sarafaabbas@gmail.com>
@sabbCodes sabbCodes force-pushed the issue-43800-target-configuration-actor branch from 98adea4 to 16a9a8b Compare April 6, 2026 07:53
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Apr 6, 2026

@mukilan mukilan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Apr 6, 2026
@mukilan mukilan added this pull request to the merge queue Apr 6, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 6, 2026
Merged via the queue into servo:main with commit c938e97 Apr 6, 2026
33 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants