Skip to content

Comments

refactor: replace function() callbacks with arrow functions#135

Merged
robertodauria merged 1 commit intom-lab:sandbox-roberto-rewritefrom
Aaradhya-07:refactor/arrow-functions-remove-self
Feb 20, 2026
Merged

refactor: replace function() callbacks with arrow functions#135
robertodauria merged 1 commit intom-lab:sandbox-roberto-rewritefrom
Aaradhya-07:refactor/arrow-functions-remove-self

Conversation

@Aaradhya-07
Copy link

The runNdt7(), runMSAK(), and runPT() methods use const self = this to capture context for nested function() callbacks. This pattern is fragile and unclear, as noted in review. This PR replaces all function() callbacks with ES6 arrow functions, which lexically bind this from the enclosing SpeedTest object. This eliminates all three const self = this assignments and every self.* reference, making the code more concise and idiomatic.

Copy link
Contributor

@robertodauria robertodauria left a comment

Choose a reason for hiding this comment

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

LGTM.

Noting that this is a PR based on top of another PR which is being actively working on by two contributors (namely, me and @bassosimone). While the changes here are good, the pattern of making changes based on other contributors' PRs without prior coordination is less than ideal, causes effort duplication, and I recommend avoiding that in the future.

On a broader note: we've been receiving a large volume of unsolicited, sometimes AI-generated PRs with no prior discussion or contact, and as a small team this is not sustainable. We are updating our contribution requirements. Going forward, please coordinate with a maintainer before opening PRs or issues. See https://github.com/m-lab/gsoc#2-engage-with-us for details.

@robertodauria robertodauria merged commit 5c23286 into m-lab:sandbox-roberto-rewrite Feb 20, 2026
2 checks passed
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.

2 participants