Skip to content

refactor: Merge log_manager crate under tedge_log_manager#3215

Merged
didier-wenzek merged 1 commit intothin-edge:mainfrom
didier-wenzek:refactor/merge-log-manager-crates
Oct 30, 2024
Merged

refactor: Merge log_manager crate under tedge_log_manager#3215
didier-wenzek merged 1 commit intothin-edge:mainfrom
didier-wenzek:refactor/merge-log-manager-crates

Conversation

@didier-wenzek
Copy link
Copy Markdown
Contributor

@didier-wenzek didier-wenzek commented Oct 30, 2024

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

@codecov
Copy link
Copy Markdown

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!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 30, 2024 •
edited
Loading

Robot Results

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

Ruadhri17
Ruadhri17 reviewed Oct 30, 2024
View reviewed changes
Copy link
Copy Markdown
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.

@didier-wenzek didier-wenzek force-pushed the refactor/merge-log-manager-crates branch from 14855ef to 1875b09 Compare October 30, 2024 11:13
Ruadhri17
Ruadhri17 approved these changes Oct 30, 2024
View reviewed changes
Copy link
Copy Markdown
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.

@didier-wenzek
Merge log_manager crate under tedge_log_manager
dca7463 
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
@didier-wenzek didier-wenzek force-pushed the refactor/merge-log-manager-crates branch from 1875b09 to dca7463 Compare October 30, 2024 12:55
@didier-wenzek didier-wenzek added this pull request to the merge queue Oct 30, 2024
@rina23q
Copy link
Copy Markdown
Member

rina23q commented Oct 30, 2024 •
edited
Loading

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
@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

Reviewers

@albinsuresh albinsuresh Awaiting requested review from albinsuresh albinsuresh is a code owner

@jarhodes314 jarhodes314 Awaiting requested review from jarhodes314 jarhodes314 is a code owner

@rina23q rina23q Awaiting requested review from rina23q rina23q is a code owner

@reubenmiller reubenmiller Awaiting requested review from reubenmiller reubenmiller is a code owner

1 more reviewer

@Ruadhri17 Ruadhri17 Ruadhri17 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@didier-wenzek @rina23q @Ruadhri17