search

phetsims / perennial

Maintenance tools that won't change with different versions of chipper checked out
MIT License
3 stars 5 forks source link

Why are we using winston? #232

Closed samreid closed 3 years ago

samreid commented 3 years ago

During https://github.com/phetsims/chipper/issues/1018, @mattpen and I discussed usage of winston. We weren't sure how much value it is adding, since the build server outputs all console.log to a file anyways. We have had hassles upgrading it in #201 and wondering if we can abandon it? But I think it is used in rosetta?

Since we aren't sure who to ask this question to, @mattpen recommended marking it for dev meeting.

jbphet commented 3 years ago

FWIW, yes, we use winston for the logging in Rosetta. I chose to use it there because it allows logging of different levels, such as 'debug', 'info', and 'error'. It also allows you to set up different transports, which are places to log information. It has been reasonably useful, but if it's giving you trouble and console.log gets you what you need, I certainly wouldn't object to not using it for the build server or whatever it is that you're using it for.

zepumph commented 3 years ago

we looked at log.js, and saw that we have never changed the debug log level on build server in many many years. No one that manages the build server seems to care about any of the features that winston gives us.

We ran out of time, but here are a few more thoughts.

It isn't used in the build server, so perhaps we can remove it from the build-server and its dependency files

It IS used in the maintenance release process, and @jonathanolson wants to keep it around for that. (see usages in Maintenance.js)

We didn't complete the discussion, but the impetus of this issue was to remove it as a dependency to annual, and perhaps we could still accomplish that if the MR process, and all of its usages of winston, are just in perennial, and not in annual.

Keeping it marked for next time.

chrisklus commented 3 years ago

From 8/5/21 dev meeting,

@samreid summarized that this is a growing pain and we are okay with it and going to close this issue.