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: host builder pipeline logging #156

Merged
merged 1 commit into from
Jul 4, 2020
Merged

Conversation

BeeHiveJava
Copy link
Contributor

Proposed change

This change moves implementations to multiple, more sensible components. It also configures SeriLog correctly in the host builder pipeline.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Currently there's a lot of logic in the Runner class, for example the loading of configuration and setting log levels. We should decouple this and load configuration at a later stage (when the app starts) so that the host builder pipeline can be as clean as possible. This change moves logging responsibilities to a different class and unifies the configuration of it. I also added some extension methods that can configure the host builder pipeline for NetDaemon correctly. The next step is to extract the configuration loading.

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the [development checklist][dev-checklist]
  • The code compiles without warnings (code quality chek)
  • Tests have been added to verify that the new code works.

This change moves implementations to multiple, more sensible components. It also configures SeriLog correctly in the host builder pipeline.
Copy link
Collaborator

@helto4real helto4real left a comment

Choose a reason for hiding this comment

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

Looks nice, I will merge it and try it on my add-on to make sure it works on dev branch.
Good job, looks much more cleaner and easier to maintain!

@helto4real helto4real merged commit a2581e6 into dev Jul 4, 2020
@helto4real helto4real deleted the feature/logging-refactor branch July 4, 2020 22:33
Ikcelaks pushed a commit to Ikcelaks/netdaemon that referenced this pull request Dec 23, 2022
This change moves implementations to multiple, more sensible components. It also configures SeriLog correctly in the host builder pipeline.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants