PretendoNetwork / account

Pretendo account server
GNU Affero General Public License v3.0
54 stars 23 forks source link

Update logger.js #68

Closed Jvr2022 closed 1 year ago

Jvr2022 commented 1 year ago

Here's what I changed:

Used fs and path built-in modules to handle file system paths and file operations

Used object destructuring to simplify the code

Used join method instead of string concatenation to build file paths

Added a LOG_DIRECTORY constant to store the path of the logs directory

Used the flags option when creating the createWriteStream to append logs to existing log files

Used toISOString() method to get the current timestamp in ISO 8601 format

Renamed input variable to message to make the code more descriptive

Used a single log function to handle all types of logs and to reduce code repetition

Used an object to store the different types of log streams and their corresponding file paths

Exported only the necessary log functions and kept the implementation details hidden.

jonbarrow commented 1 year ago

As stated in https://github.com/PretendoNetwork/account/pull/67 you are working with the wrong branchs, and given that you removed several key dependencies in https://github.com/PretendoNetwork/account/pull/67 I'm not sure you are testing any of this code, and therefore I am not comfortable merging any of it

To comment on this PR specifically:

  1. fs-extra is used because it is a drop-in replacement for the built-in fs with extra features. We use it elsewhere in the codebase for those features. In Node when a module is imported it is cached and reused any time it gets imported. By importing both fs-extra and fs like you have, you have essentially doubled the needed memory for a single module
  2. The colors module in the logger does not have any fields called success, error, warn, or info which would in a crash whenever anything is logged, since the function you are trying to call does not exist. The colors module adds fields to the String primitive for managing colors, hence why we used it that way