Skip to content

Turn Queue into a class#1311

Merged
matux merged 2 commits into
masterfrom
matux/modernize-queue
Sep 18, 2025
Merged

Turn Queue into a class#1311
matux merged 2 commits into
masterfrom
matux/modernize-queue

Conversation

@matux

@matux matux commented Sep 18, 2025

Copy link
Copy Markdown
Contributor

Description of the change

This pull request refactors the Queue class in src/queue.js from a function-based prototype implementation to a modern ES6 class. The update improves code readability, maintainability, and consistency by using class syntax, arrow functions, and updated variable declarations. The changes also include minor improvements to documentation and method signatures.

These changes collectively modernize the codebase, making it easier to maintain and extend in the future.

@matux matux requested a review from Copilot September 18, 2025 18:28
@matux matux self-assigned this Sep 18, 2025

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modernizes the Queue implementation by converting it from a function-based prototype pattern to an ES6 class. The refactoring improves code readability and maintainability while preserving all existing functionality.

Key changes:

  • Converted function constructor and prototype methods to ES6 class syntax
  • Updated variable declarations from var to const/let for better scoping
  • Replaced anonymous functions with arrow functions for cleaner syntax

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/queue.js
Comment on lines +275 to +276
originalError ||
_.get(data, 'body.trace.exception.message') ||

Copilot AI Sep 18, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] The logical OR chain assignment pattern is less readable than the original sequential assignment with fallback checks. Consider reverting to the original pattern or using nullish coalescing operator (??) if null/undefined are the only falsy values you want to handle.

Suggested change
originalError ||
_.get(data, 'body.trace.exception.message') ||
originalError ??
_.get(data, 'body.trace.exception.message') ??

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I prefer the PR version.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, thought about it, maybe it's right to use ?? maybe not, but what's sure is that it would change behavior, and it's not something this PR intends to do.

Comment thread src/queue.js
@matux matux enabled auto-merge (squash) September 18, 2025 19:53
@matux matux merged commit 49d4eca into master Sep 18, 2025
4 checks passed
@matux matux deleted the matux/modernize-queue branch September 18, 2025 19:55
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