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

Clean up the logging #11

Closed
FelixGV opened this issue Nov 27, 2014 · 5 comments
Closed

Clean up the logging #11

FelixGV opened this issue Nov 27, 2014 · 5 comments

Comments

@FelixGV
Copy link
Contributor

FelixGV commented Nov 27, 2014

System.out and System.err is pretty messy...

We should use a proper logging lib, though I don't know which one in particular.

@eric239
Copy link

eric239 commented Nov 28, 2014

Hi @FelixGV
I'm interested in using schema-repo and would like to contribute, and this issue sounds like something I can tackle.
I would suggest using slf4j as the most "implementation neutral" framework.
Is this someone already working on this issue? If not, I can fork and then submit a pull request.
Please let me know.
Thanks.

@FelixGV
Copy link
Contributor Author

FelixGV commented Nov 28, 2014

Hi @eric239,

I'm glad to hear you're interested in using and contributing to the schema repo (:

I think you're right. SLF4J is the right API for the schema repo because we would like other developers to be able to easily integrate the repo (or at least its client) in their projects no matter which logging library they wish to use.

That being said, at least for the bundle modules, we should provide a default logging implementation which the SLF4J library would leverage. Have you thought about which default lib you would like to use? Logback seems to be pretty solid and to work well with SLF4J.

Please let me know what you think, and definitely feel free to fork and submit a PR!

Thanks (:

-F

@eric239
Copy link

eric239 commented Nov 28, 2014

Yep, I agree - Logback is de-facto top choice.

@FelixGV
Copy link
Contributor Author

FelixGV commented Nov 28, 2014

Great!

BTW, you may want to base your work off of the fix_issue_7 branch on my repo.

It contains an extra commit for Issue #7 that I'm planning to merge in the shared master next week. I'm leaving it as an un-merged PR for now because I'd like to foster a culture of reviews in this project and let some time for people to react if they see anything wrong. But since I'm pretty much the only contributor at the moment, I'm still going to merge it in in a few days if no one expresses any feedback.

In any case, if you prefer working off of the shared master and merging with my other changes later on, that's fine too.

Along the same train of thought, I think it might be good for you to ask some review on the mailing list once you submit your PR. Eventually, we should probably write a How to Contribute wiki page, but maybe not quite yet, since I'm not exactly sure what the process should look like...

Thanks for the help!

-F

eric239 pushed a commit to eric239/schema-repo that referenced this issue Nov 30, 2014
FelixGV pushed a commit to FelixGV/schema-repo that referenced this issue Dec 2, 2014
@FelixGV
Copy link
Contributor Author

FelixGV commented Dec 2, 2014

The fix for this has been merged.

Thanks!

@FelixGV FelixGV closed this as completed Dec 2, 2014
FelixGV pushed a commit to FelixGV/schema-repo that referenced this issue Dec 9, 2014
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

No branches or pull requests

2 participants