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

Modularize: separate the core SLF4J binding from the optional handlers #21

Merged
merged 14 commits into from
May 14, 2024

Conversation

azabost
Copy link
Member

@azabost azabost commented May 3, 2024

The main purpose of this PR is to modularize this library so that the core feature, which is SLF4J binding, can be used without the burden of unnecessary dependencies such as the FileLogHandler and the NotifyDeveloperHandler.

Here's a summary of the changes:

  • Split app module into: core, handler-file-log, handler-notify-developer
  • Updated README to reflect the changes in modules (e.g. an additional dependency is required to access the FileLogHandler)
  • Added an auxiliary testapp application module that can be used for testing the other modules
  • Updated Gradle and Android Gradle Plugin to the latest versions (8.7 and 8.4.0)
  • Lowered Java sourceCompatibility and targetCompatibility back to 1.8 (my previous PR left them undefined hence they were the same as the configured Java toolchain, which was 17)
  • Removed a few @TargetApi occurrences because they were no longer relevant due to a higher min SDK version
  • To accommodate the module reorganization, I had to make the Disposable interface public and expose a public LoggerConfiguration.registerDisposable method. These changes were required only to let NotifyDeveloperHandler work with as few changes as possible. Not sure if we want to keep that functionality in the long run, but I didn't want to remove it just yet. If we decided to get rid of that handler, we could also remove the new public method and the Disposable interface.

@azabost azabost requested a review from miensol May 3, 2024 11:05
try {
closeable.close();
} catch (IOException e) {
Log.e(TAG, "Could not close " + closeable, e);
Copy link
Member Author

@azabost azabost May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log.e is used for consistency with the existing code in this class. Nevertheless, I don't see a reason why SLF4J API shouldn't be used here, so we might as well migrate all Log.e to Logger.error sooner or later.

@azabost azabost merged commit 427e52b into bright:master May 14, 2024
1 check passed
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