The first PR to lovely Telescope

Picking the issue 

 After watching my professor's video A Tour of the Telescope Project and followed through Environment Setup documentationfinally I am able to setup Telescope on my machine successfully, and tried to do my first PR to Telescope project.

When I browsing the existing issue list, I found one Issue-1264 which mentioned in Week 7's video Fixing a bug in Telescope still there. I always afraid that I might fail to fix a back-end bug, so I have tried it before I  ask for the assignee. 

Fix the bug that the setting of LOG_LEVEL in .env is not working properly. Even set LOG_LEVEL=debugnpm start still print as LOG_LEVEL=info

Working on the issue

Firstly, I tested the bug by set LOG_LEVEL to debug in .env and confirmed that it only consoled the level of logs. Then I need to find which file consoled those logs. After scan over the back end files. I found logger.js inside utils folder is responded to consoling the  LOG_LEVEL.


require('../lib/config');

// Deal with log levels we don't know, or which are incorrectly formatted
let logLevel = (process.env.LOG_LEVEL || 'info').toLowerCase();
if (!pino.levels.values[logLevel]) {
  // Use `debug` by default in development mode, `info` otherwise.
  logLevel = process.env.NODE_ENV === 'development' ? 'debug' : 'info';
}


 I used console.log() to console process.env.LOG_LEVEL, it still prints "info". That means this file does not receive the setting from .env file. I treplaced require('../lib/config') to require('dotenv')config and it prints debug logs now.  

Sending the pull request

After my professor viewed my pull request, he pointed out that we do include the dotenv in lib/config. So there should be something wrong inside lib/config. Everything in lib/config looks fine. When I googled it, I Found a similar issue online. It says "By requiring .en config at the top,logger.ts will have theLOG_LEVELenvironment variable.".

After my testing, it works as long asdotenv.config()before logger dependency. I followed Professor Humphrey's  suggestion to rip theloggerdependency out of lib/config.js and use console.log() in lib/config file.


After everything is done, my pull request still blocked by failed the TravisCI issue and the macOS issue. I would take a look when I finish my other pull request, but I believed that someone else would pick them up and fixed them by the time. 




















Comments

Popular posts from this blog

My PR for release 0.3 External Project

OSD600 Lab8

OSD600 -- Release 0.4.1 -- Choose Project and issues