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.

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

Refactor project #15

Merged
merged 1 commit into from
Feb 20, 2024
Merged

Refactor project #15

merged 1 commit into from
Feb 20, 2024

Conversation

Americas
Copy link
Collaborator

@Americas Americas commented Jan 2, 2024

Update the code style to more modern standards.

Notable changes:

  • writer argument signature now conforms to docs, it will accept a string address or an object with an optional nsqd property (passing nsqlookupd is now ignored since it doesn't work, and passing host and port is ingnored since it was not documented, use the string format if desired)
  • added a subscribed event on the Reader class (used on a test to avoid using setTimeout to wait for setup).
  • removed should dev dependency.
  • added test runner

Closes #11

@Americas Americas requested a review from kurayama January 2, 2024 15:43
@Americas Americas self-assigned this Jan 2, 2024
@Americas Americas changed the title Refactor Framer class Refactor project Jan 2, 2024
@Americas Americas force-pushed the support/refactor-project branch 10 times, most recently from b0a9507 to 7be5b8d Compare January 9, 2024 16:57
lib/utils.js Outdated
return {
host: str.split(':')[0],
port: str.split(':')[1]
exports.parseAddress = str => {
Copy link

Choose a reason for hiding this comment

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

Small inconsistency: we are using exports and in other places module.exports. Do we want to standardize it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

- Add linter
- Refactor `Framer` class
- Refactor `Connection` class
- Refactor `Message` class
- Refactor utils
- Refactor `Reader` class
- Refactor `Writer` class
- Refactor mixins
- Refactor examples
- Refactor index.js
- Update dependencies
- Refactor Readme.md
- Add minimum node engine version
- Add test github action
@satazor satazor merged commit f53ac27 into master Feb 20, 2024
1 check passed
@Americas Americas deleted the support/refactor-project branch May 21, 2024 16:03
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.

Change URL of nsq-lookup dependency to not require authentication
2 participants