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

Add a new log option to createAppAuth(options), then replace console.warn with state.log.warn #176

Closed
gr2m opened this issue Sep 15, 2020 · 2 comments
Labels
Type: Feature New feature or request

Comments

@gr2m
Copy link
Contributor

gr2m commented Sep 15, 2020

We are trying to avoid any direct calls to console, so that consumers of the Octokit libraries can use their own logging tool. The way we do that is a log option that is compatible with the global console object. It should at least set { debug, info, warn, error }. By default, { debug, info } are no-ops, the other two default to console.warn and console.error respectively. Compare https://github.com/octokit/core.js#logging / https://github.com/octokit/core.js/blob/37437bcf632ce32ba25663e575de96a844b802b2/src/index.ts#L111-L119

We introduced console.warn statements in #164. These should be replaced with what ever is passed as options.log.warn to the createAppAuth method.

I'd happily accept a pull request and will provide guidance if you are interested in contributing. The pull request should add a test (or adjust an existing one), it should implement the feature, and add the new option to the documentation in README.md

@gr2m gr2m added the Type: Feature New feature or request label Sep 15, 2020
@romanbalayan
Copy link
Contributor

Hi @gr2m,
I opened a PR for this. I'm still quite unsure how to go about testing the log methods - debug, info, and error. Currently they are not covered on the test cases.

@gr2m
Copy link
Contributor Author

gr2m commented Oct 15, 2020

thanks @romanbalayan, great work! resolved via #190

@gr2m gr2m closed this as completed Oct 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants