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

F nolog4js #59

Merged
merged 3 commits into from
Jul 28, 2017
Merged

F nolog4js #59

merged 3 commits into from
Jul 28, 2017

Conversation

philals
Copy link
Contributor

@philals philals commented Jul 23, 2017

This removes log4js.

I removed it and copied into my node_modues to see if it resolved my webpack issues. It did. And tests are green locally.

@philals philals mentioned this pull request Jul 23, 2017
@jordanwalsh23
Copy link
Contributor

Awesome. Can't wait to test and merge.

Is this a breaking change?

@philals
Copy link
Contributor Author

philals commented Jul 23, 2017

@jordanwalsh23 Excellent.

It will be a breaking change for anyone who is depending on the logging. But as far as I can tell there was no way to configure logging via the config passed into xero-node. So I think that it was always just logging to stout anyway.

I'm hoping that no one is dependent.

The early days are the ones to make theses changes. See #52 for some of my thoughts on future.

…nolog4js

# Conflicts:
#	docs/Sample-App-Setup.md
#	sample_app/index.js
@philals
Copy link
Contributor Author

philals commented Jul 25, 2017

@jordanwalsh23 uptodate with master and green locally

Cheers

@jordanwalsh23 jordanwalsh23 self-assigned this Jul 27, 2017
@jordanwalsh23 jordanwalsh23 merged commit a0ce47b into XeroAPI:master Jul 28, 2017
jordanwalsh23 added a commit that referenced this pull request Aug 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants