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 third pull request for Hacktoberfest

OSD Lab3 Git Merge

My PR for release 0.3 External Project