Skip to content

feat: add TailFileStream #1

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 4 commits into from
Mar 13, 2024
Merged

feat: add TailFileStream #1

merged 4 commits into from
Mar 13, 2024

Conversation

ivan-tymoshenko
Copy link
Member

No description provided.

index.js Outdated
Comment on lines 69 to 70
this.emit('open', this.#fd)
this.emit('ready')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this.emit('open', this.#fd)
this.emit('ready')
process.nextTick(() => {
this.emit('open', this.#fd)
this.emit('ready')
})

See nodejs/node#51993

Co-authored-by: Matteo Collina <matteo.collina@gmail.com>

stream.on('data', () => {})
stream.on('end', () => {
stream.close(done)
Copy link
Member

Choose a reason for hiding this comment

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

calling stream.close(done) should not be necessary, it should auto-close on 'end' or 'close'.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added this test to test this line behavior:

if (typeof cb === 'function') finished(this, cb)

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@ivan-tymoshenko ivan-tymoshenko merged commit e78268d into main Mar 13, 2024
12 checks passed
@ivan-tymoshenko ivan-tymoshenko deleted the add-tail-file-stream branch March 13, 2024 14:23
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants