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

Separate source/header files, bug fixes… #453

Merged
merged 1 commit into from
Jan 8, 2017
Merged

Separate source/header files, bug fixes… #453

merged 1 commit into from
Jan 8, 2017

Conversation

aparajita
Copy link
Contributor

  • Non-templated, non-trivial or non-performance critical code is all in a separate source file.
  • Replaced ELPP_STACKTRACE_ON_CRASH (which wasn’t used) with ELPP_FEATURE_CRASH_LOG.
  • Small fixes for compiler warnings.

Please note that the code is working. I'm making the PR now so you can make sure all of the tests are working. There is still much to do before releasing this:

  • Projects to create static and dynamic libs. I have already done this for Xcode, I'll be pushing that soon.
  • Update the README for the new way of using the lib. I'm working on this. Ultimately I'd like to substantially edit the docs to be easier to use.
  • Sample files that illustrate usage of source, static lib, and dynamic lib. I have an Xcode project that does this.
  • Sample file that illustrates all of the possible uses of CustomFormatSpecifier. Still working on this.

- Non-templated, non-trivial or non-performance critical code is all in a separate source file.
- Replaced ELPP_STACKTRACE_ON_CRASH (which wasn’t used) with ELPP_FEATURE_CRASH_LOG.
- Small fixes for compiler warnings.
@abumq
Copy link
Owner

abumq commented Jan 8, 2017

I think a lot of more code can be moved to cc file such as initialize easyloggingpp and user will not need to do this. But i am happy so far. Greatly thanks for this

@abumq abumq merged commit 25499f2 into abumq:elpp_single_header_changes Jan 8, 2017
@aparajita
Copy link
Contributor Author

The call to init elpp is in the cc file now, but I guess all of those macros can go away now. If you eliminate those macros, then you don't need to check ELPP_AS_DLL in the cc file, the init can always be done there and then the user doesn't need to.

There isn't much more code that can be moved to the cc. I went over line pretty carefully. Everything that I left in the header is either:

  • Templated code that must remain in the header.
  • Trivial code that won't cause code bloat by being inlined.
  • Performance critical code that will benefit from inlining.

One important thing that will have to be noted in the docs: there are some config macros that will significantly change the code in a static or dynamic lib. It may be necessary to have different versions, for example thread-safe vs. non-thread-safe. I'll leave it to you to figure out which macros will have this effect.

@aparajita aparajita deleted the elpp_single_header_changes branch January 15, 2017 06:40
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