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

refactor!: drop runTask method #363

Merged
merged 4 commits into from
Sep 21, 2023
Merged

refactor!: drop runTask method #363

merged 4 commits into from
Sep 21, 2023

Conversation

metcoder95
Copy link
Member

Context

The PR attempts to drop the runTask method signature as part of the new v5 work.

Changes

  • refactor: replace abortcontroller lib with native
  • refactor!: remove deprecated runTask method

@metcoder95 metcoder95 marked this pull request as draft June 19, 2023 21:06
@metcoder95 metcoder95 changed the base branch from current to next June 30, 2023 11:49
@metcoder95 metcoder95 marked this pull request as ready for review September 13, 2023 12:01
RafaelGSS
RafaelGSS reviewed Sep 20, 2023
View reviewed changes
Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

Can you elaborate on why should we do it? I might be missing something due to context switching.

@metcoder95
Copy link
Member Author

Piscina#runTask has been marked as deprecated for a while. According to the commit history, it seems that we wanted to reduce the number of positional arguments and opted for an object passed as a second argument instead. See: a58f99a

I'm pointing to next and seeking to remove the whole method

RafaelGSS
RafaelGSS approved these changes Sep 20, 2023
View reviewed changes
Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

LGTM!

@metcoder95 metcoder95 merged commit 7b86105 into next Sep 21, 2023
10 checks passed
This was referenced Jul 26, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RafaelGSS RafaelGSS RafaelGSS approved these changes

Assignees
No one assigned
Labels
semver-major
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@metcoder95 @RafaelGSS