search

processing / p5.js-web-editor

The p5.js Editor is a website for creating p5.js sketches, with a focus on making coding accessible and inclusive for artists, designers, educators, beginners, and anyone else! You can create, share, or remix p5.js sketches without needing to download or configure anything.
https://editor.p5js.org
GNU Lesser General Public License v2.1
1.39k stars 1.33k forks source link

Add logging with winston #853

Open siddhant1 opened 5 years ago

siddhant1 commented 5 years ago

I think we should remove console.logs from the code

Links :- https://dev.to/mornir/-dont-leave-console-logs-in-production-14na https://hackernoon.com/please-stop-using-console-log-its-broken-b5d7d396cf15

I can send in a PR

Have a look @catarak

catarak commented 5 years ago

totally! it would be amazing to add a logging library. maybe this ticket could be that—investigate and add logging library?

siddhant1 commented 5 years ago

Yeah , I will look into it and start refactoring

siddhant1 commented 5 years ago

As I was looking through the code , I found out that most of the console statements were used for debugging and some for handling errors. There are 2 ways to solve this.

  1. Remove console.logs used for debugging 2.find a way to disable console.logs on the prod mode and enable in dev mode which agains boils down to adding a logging library
catarak commented 5 years ago

sounds great to me!

siddhant1 commented 5 years ago

@catarak I was thinking of a solution to this ticket. Should I monkey patch window.console.log which checks if we are in dev mode or prod mode and logs only in dev mode?

catarak commented 5 years ago

process.env.NODE_ENV is already passed to the front end (e.g. in App.jsx), so removing console logs when in prod should be pretty simple!

GaurangTandon commented 5 years ago

@siddhant1 Could I please work on this issue while you're working on the other ones? Thank you!

siddhant1 commented 5 years ago

Hey Gaurang , this issue was already solved and needs to be closed. You can work on other ones :)

catarak commented 5 years ago

@siddhant1 / @GaurangTandon this issue hasn't been solved. no logging library has been added.

siddhant1 commented 5 years ago

The issue was originally to remove logs which was done , for adding logging library , I will send a pr soon