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

Refactor Scrolls #71

Merged
merged 17 commits into from
Sep 4, 2017
Merged

Refactor Scrolls #71

merged 17 commits into from
Sep 4, 2017

Conversation

asenchi
Copy link
Owner

@asenchi asenchi commented Aug 23, 2017

This commit breaks backwards compatibility for Scrolls. It is a rework/refactor of the initial work done in the following PR: #54

  • The #global_context is no longer mutable, instead we instantiate a class internally inside Scrolls::Logger that contains the global_context. This is to avoid various bad thread behaviors, that previous to Ruby 2.0, were allowed.
  • The result of the above change removes #add_global_context.
  • Currently Scrolls needs to be instantiated to be used (Scrolls.init(options)) however I'd like to work on a method for allowing one to just use Scrolls.log and get sane defaults.
  • Due to this massive change I moved to "single line exceptions" by default since it lowers the amount of data produced by Scrolls when using #log_exception without losing the information.
  • Officially kills Ruby 1.8.7 support (though literally no one should be using that version).

NOTE: I don't have a great setup right now to test everything here. I'm testing against Ruby 2.4.1 and all tests pass. Please test if you can. I'm going to cut a release of this version, 0.9.0.pre. I'm jumping versions here since we break backward compatibility. I'm hopeful with this and subsequent versions we can get to a place where a version 1.0 is a reality.

Please test this version as much as possible!

This commit breaks backwards compatability for Scrolls. It
is a rework/refactor of the initial work done in the
following PR: #54

- The `global_context` is no longer mutable, instead we
instantiate a class internally inside Scrolls::Logger that
contains the `global_context`. This is to avoid various bad
thread behaviors, that previous to Ruby 2.0, were allowed.
- The result of the above change removes `#add_global_context`.
- Currently Scrolls needs to be instantiated to be used
(`Scrolls.init(options)`) however I'd like to work on a
method for allowing one to just use `Scrolls.log` and get
sane defaults.
- Due to this massive change I moved to "single line
exceptions" by default since it lowers the amount of data
produced by Scrolls when using `#log_exception` without
losing the information.
@asenchi
Copy link
Owner Author

asenchi commented Aug 30, 2017

This evening I worked to clean up a lot of the refactor, simplify some areas (especially around streams, facilities and structure).

Next I want to bring in any other PRs that existed, make sure they are well tested, appropriate and then work on updating all of the documentation. Then I think this is ready for a proper release.

I also added some common log level abbreviations to make
this easier to use. Tests included.
Closes #67.

Add an option to Scrolls#init "escape_keys" that escapes
various characters in keys. Implementation is based off how
Rack escapes html. I started there, but we can extend the
character range further if need be.

Also some code cleanup, restructuring and comments.
Fixes #61

In Ruby 2.1 setting thread local values changed. This
updates our usage.
This not only makes more sense but "bare" hashes are not
allowed for arguments in recent versions of Ruby so this
will break anyways.
@asenchi
Copy link
Owner Author

asenchi commented Sep 4, 2017

Alright, I've updated docs, cleaned up the code, added some tests and overall is am ready to merge this and release 0.9.0.

@asenchi asenchi merged commit a807d54 into master Sep 4, 2017
@asenchi asenchi deleted the refactor branch September 4, 2017 14:38
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.

1 participant