Skip to content

Conversation

@taniishkaaa
Copy link
Contributor

Replaces CanGc::note() calls with arguments passed by callers in components/script/dom/errorevent.rs


@taniishkaaa taniishkaaa requested a review from gterzian as a code owner October 21, 2024 22:18
Copy link
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

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

Very nice! Who knew we fired so many error events?

@jdm jdm enabled auto-merge October 21, 2024 23:56
line_number: u32,
fetch_options: ScriptFetchOptions,
script_base_url: ServoUrl,
can_gc: CanGc,
Copy link
Member

Choose a reason for hiding this comment

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

We need to allow this method to avoid the lint error:

error: this function has too many arguments (8/7)
    --> components/script/dom/globalscope.rs:2633:5
     |
2633 | /     pub fn evaluate_script_on_global_with_result(
2634 | |         &self,
2635 | |         code: &SourceCode,
2636 | |         filename: &str,
...    |
2641 | |         can_gc: CanGc,
2642 | |     ) -> bool {

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'll open a new PR for that?

Copy link
Member

Choose a reason for hiding this comment

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

No, it can be addressed in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, on it!

Signed-off-by: taniishkaaa <tanishkasingh2004@gmail.com>
Signed-off-by: taniishkaaa <tanishkasingh2004@gmail.com>
auto-merge was automatically disabled October 22, 2024 09:35

Head branch was pushed to by a user without write access

@taniishkaaa taniishkaaa requested a review from mukilan October 22, 2024 09:36
@mukilan mukilan enabled auto-merge October 22, 2024 09:56
@mukilan mukilan added this pull request to the merge queue Oct 22, 2024
Merged via the queue into servo:main with commit 7015e0f Oct 22, 2024
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