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: Merge log_manager crate under tedge_log_manager #3215

Merged

Conversation

didier-wenzek
Copy link
Contributor

Proposed changes

The log_manager crate being only used by tedge_log_manager, it's better to make this clear and merge these two crates.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue


Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s)
  • I ran cargo fmt as mentioned in CODING_GUIDELINES
  • I used cargo clippy as mentioned in CODING_GUIDELINES
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

Copy link

codecov bot commented Oct 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Additional details and impacted files

📢 Thoughts on this report? Let us know!

Copy link
Contributor

github-actions bot commented Oct 30, 2024

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
526 0 2 526 100 1h36m51.680663s

Copy link
Contributor

@Ruadhri17 Ruadhri17 left a comment

Choose a reason for hiding this comment

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

Looks good but crates in cargo.toml must be sorted to please taplo.

Copy link
Contributor

@Ruadhri17 Ruadhri17 left a comment

Choose a reason for hiding this comment

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

Approved.

@rina23q
Copy link
Member

rina23q commented Oct 30, 2024

Thanks for detecting this. The log_manager crate was used when we had supports both for the legacy c8y_log_plugin and the new tedge_log_manager. Since c8y_log_plugin is deprecated long time ago, the common crate log_manager no longer makes sense.

Merged via the queue into thin-edge:main with commit 1a27e54 Oct 30, 2024
33 checks passed
@didier-wenzek didier-wenzek deleted the refactor/merge-log-manager-crates branch October 30, 2024 13:52
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.

3 participants