Skip to content

chore: convert src/logging.{js,ts} #1907

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bkeepers
Copy link
Contributor

As a way of getting familiar with the SignalK internals, I'm working on converting some of the remaining code to TypeScript. This converts src/logging.js to TypeScript.

@tkurki
Copy link
Member

tkurki commented Mar 25, 2025

Would it make sense to compile a single PR instead of PR per each file? Do you have a plan of attack for the other files?

@bkeepers
Copy link
Contributor Author

Would it make sense to compile a single PR instead of PR per each file? Do you have a plan of attack for the other files?

Sure I can batch some together if you prefer. I don't really have a plan besides ls -Sr src/**/*.js to get the list of files remaining sorted smallest to largest and started on the first one I saw that didn't have other dependencies, but I can put together a little more of a plan and do a larger chunk.

@bkeepers
Copy link
Contributor Author

Also, I converted the signalk-tides plugin to TypeScript and ran into a lot of challenges that I plan to open PRs to address: bkeepers/signalk-tides#2

@tkurki tkurki added the chore label Mar 30, 2025
process.stderr.write = function (string) {
errWrite.apply(process.stderr, arguments)
storeOutput(string, true)
process.stderr.write = function (...args) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why aren't you doing function (...args: Parameters<typeof errWrite>) here? Similarly for outWrite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants