disable logging in production.

[distalx]

This will disable console.log when app is running in production.

[angelocordon]

(Mentioned this in the Discord channel, pardon the duplication, but here’s a more expanded view)

I’m not too keen on overriding default dev tools in order to prevent console.logs from leaking. I see use cases where we need to use console.log in production in order to debug things.

I think what might be better instead is to set up tools so that we don’t leak our console logs - using ESLint for example, and failing a PR so we don’t merge it into a production branch.

What do you think?

[lpatmo]

@angelocordon I'm confused about your suggestion to use ESLint to prevent console logs in production, when you said that we might want console.logs in production occasionally. When would we need the console logs in production? And how would we prevent console logs from being stopped by whatever tool we are using to stop console logs?

[angelocordon]

@lpatmo - at the root of it, the problem isn't that we're using console.logs, it's that we're committing console.logs into branches and PRs.

For the record, I'm totally okay with us using console when needed, but we shouldn't be leaving them behind both from a code quality standpoint and developer productivity standpoint when checking our work in, or merging our PRs.

Tools like ESLint should comb our codebase and warn us of patterns like these so that we can prevent console.logs from leaking in either environments. The tasks on #40 would help report back on these kinds of things so that we know what we're merging into master or staging, thus hopefully preventing us from leaking console.log in the first place.

To my understanding, the changes on this PR prevents anybody from returning anything relevant when using console. Sometimes you need to debug right in the browser to have a deeper understanding of an issue, or write out some functions right on the console window (I've done this a few times in the past).

-- Sorry for the long reply, just want to make sure that I'm articulating my thoughts clearly :) If we're all good with this, we can go ahead and close the PR :)

[lpatmo]

Thanks for explaining your thought process! My understanding of the PR is different:

prevents anybody from returning anything relevant when using console. Sometimes you need to debug right in the browser to have a deeper understanding of an issue, or write out some functions right on the console window (I've done this a few times in the past).

That is, it will prevent stray console logs from showing up production, but doesn't prevent us from writing out functions on the console window, and if we needed to see console logs on the server, we could use a staging environment.

[angelocordon]

Perhaps @distalx can chime in here as well, but to my understanding: https://github.com/codebuddies/cb-connect/blob/31ce98162108b19b394e3fd040d4d42ad7e17b70/client/main.js#L8-L9

console ends up being assigned the console object or an empty object if console is undefined. console.log then gets reassigned as an empty function.

If you try doing the same code in your console, and then try console.logging something, it returns as undefined. (in this example, normal console.log should return x)

So I think it will still return something, we'll just have multiple undefined in the console instead.

if we needed to see console logs on the server, we could use a staging environment.

staging and production might not always be in sync :) Typically, staging would house future releases not out to the public yet.

-- Of course, I might be totally missing something here as well - do we have any value in using this code and is it fixing the right problem?

[distalx]

Some clarification.

This only overrides the log api in production mode.

Other APIs are still useable in production mode. i.e. info, warn, error, debug, dir and few others.

My initial thought process was to not be strict on logs, I personally sprinkle few when working with new tech. And newbies or people who are contributing for first time don't get the impression that we are perfectionist.

Thanks all, for sharing your thoughts, It encourage me to do some reading and I learned that this is not a standard solutions, but a practical one.

On Wed 3 Apr, 2019, 05:08 Linda, notifications@github.com wrote:

Thanks for explaining your thought process! My understanding of the PR is different:

prevents anybody from returning anything relevant when using console. Sometimes you need to debug right in the browser to have a deeper understanding of an issue, or write out some functions right on the console window (I've done this a few times in the past).

That is, it will prevent stray console logs from showing up production, but doesn't prevent us from writing out functions on the console window, and if we needed to see console logs on the server, we could use a staging environment.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/codebuddies/cb-connect/pull/44#issuecomment-479256184, or mute the thread https://github.com/notifications/unsubscribe-auth/AKrNwNAWmyCa964bt8ZI5IFYPnYAK9dHks5vc-oOgaJpZM4cYRgc .

[distalx]

Also, This won't return undefined. We have very same setup for CB. Where your logs will be printed in locally but won't appear in production. Not even an undefined. This modifies log API into empty function and nothing outputs when one calls an empty function.

On Tue 2 Apr, 2019, 22:36 Angelo Cordon, notifications@github.com wrote:

@distalx https://github.com/distalx - any particular reason why we should do this? Curious to hear your perspective.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/codebuddies/cb-connect/pull/44#issuecomment-479097756, or mute the thread https://github.com/notifications/unsubscribe-auth/AKrNwNLqluU5XPm9bfoOITmvF1yEPFzNks5vc44FgaJpZM4cYRgc .

[distalx]

Deviated from the original intent of the pull request. So closing.