Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify logging #95

Closed
Esemesek opened this issue Jan 16, 2019 · 12 comments
Closed

Unify logging #95

Esemesek opened this issue Jan 16, 2019 · 12 comments
Assignees
Labels
good first issue Good for newcomers
Milestone

Comments

@Esemesek
Copy link
Member

Unify logging through-out the cli. We will need a couple of levels of logging:

  • ERROR
  • WARNING
  • INFO
  • DEBUG - we will need it for --verbose logging

Could be very similar to Haul's logger: https://github.com/callstack/haul/blob/next/src/logger.js

@Esemesek Esemesek added this to the Better UX milestone Jan 18, 2019
@Esemesek Esemesek self-assigned this Jan 23, 2019
@Esemesek
Copy link
Member Author

Esemesek commented Jan 23, 2019

I've added simple logger (cli/src/util/logger) to the project. It should be okay for now. We still need to replace all the console.log, console.warn, console.error and npmlog:

  • - cli/src/* (taken)
  • - cli/src/bundle
  • - cli/src/core
  • - cli/src/dependencies - We are dropping this, so no need to add custom logging here
  • - cli/src/eject
  • - cli/src/generator
  • - cli/src/info (taken)
  • - cli/src/init (taken)
  • - cli/src/install (taken)
  • - cli/src/library (taken)
  • - cli/src/link
  • - cli/src/logAndroid
  • - cli/src/logIOS
  • - cli/src/runAndroid
  • - cli/src/runIOS
  • - cli/src/server
  • - cli/src/util

If anyone is interested in grabbing something just write it here.

@sidferreira
Copy link

sidferreira commented Feb 3, 2019

@Esemesek I want to help on this one. Should I do it in a fork or wait to be assigned?

@Esemesek
Copy link
Member Author

Esemesek commented Feb 4, 2019

@sidferreira just tell me what part can you take, and I'll assign it to you C:

@sidferreira
Copy link

@Esemesek is there a base branch for it? I may be wrong but it sounds like a quick task if done with regular expressions...

@thymikee
Copy link
Member

thymikee commented Feb 4, 2019

There's no branch, we just file small PRs to not disrupt others work too much :) Since there's not much left, feel free to send a PR with all of it 👍

@sidferreira
Copy link

@thymikee Just to be sure: only on packages/cli/src right?

@thymikee
Copy link
Member

thymikee commented Feb 4, 2019

Yup!

@sidferreira
Copy link

@thymikee @Esemesek seem that npmlog isn't in use anymore

@thymikee
Copy link
Member

thymikee commented Feb 4, 2019

Oh, that's cool! But there should be some regular console.logs that would be nice to convert to universal logger

@sidferreira
Copy link

@thymikee yeah, I replaced any console.something

@sidferreira
Copy link

@thymikee close?

@thymikee
Copy link
Member

thymikee commented Feb 4, 2019

Awesome work @sidferreira, thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants