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

fix: Remove slf4j plugin dependency #822

Merged
merged 2 commits into from
Sep 21, 2023
Merged

fix: Remove slf4j plugin dependency #822

merged 2 commits into from
Sep 21, 2023

Conversation

kolipakakondal
Copy link
Collaborator

@kolipakakondal kolipakakondal commented Sep 20, 2023

Helps to fix Eclipse 2023-09 update error

Description

Remove slf4j plugin dependency

Fixes # (IEP-1025)

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How has this been tested?

  • Install PR update site build on Eclipse 2023-09 and Eclipse 2023-06,

Test Configuration:

  • ESP-IDF Version:
  • OS (Windows,Linux and macOS):

Dependent components impacted by this PR:

  • Updat site

Checklist

  • PR Self Reviewed
  • Applied Code formatting
  • Added Documentation
  • Added Unit Test
  • Verified on all platforms - Windows,Linux and macOS

Summary by CodeRabbit

  • Refactor: Updated dependencies in com.espressif.idf.terminal.connector.serial project. Removed dependency on org.slf4j.api and added a new dependency on org.eclipse.ui.console for enhanced console functionality.
  • Chore: Changed the repository location for org.eclipse.cdt.lsp.feature.feature.group in com.espressif.idf.target project. The new location points to the stable release of CDT Language Server Protocol (LSP) instead of the master build, improving reliability.

Helps to fix Eclipse 2023-09 update error
@coderabbitai
Copy link

coderabbitai bot commented Sep 20, 2023

Walkthrough

This pull request introduces a shift in the project dependencies and updates the repository location for a feature group. The changes primarily affect the terminal connector serial bundle and the target definition file.

Changes

File Summary
.../connector.serial/META-INF/MANIFEST.MF Removed org.slf4j.api and added org.eclipse.ui.console to the Require-Bundle section, indicating a change in dependencies.
.../idf.target/com.espressif.idf.target.target Updated the repository location for org.eclipse.cdt.lsp.feature.feature.group from a master build to a specific release.

🐇💻

In the land of code, where the semicolons lie,

Dependencies shift, under the digital sky.

From slf4j we part, console UI we embrace,

And CDT's LSP finds a new base.

A rabbit hops through, leaving tracks in its wake,

Celebrating the changes that developers make. 🎉


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Commits Files that changed from the base of the PR and between 6ff9814 and c87d397.
Files selected for processing (1)
  • bundles/com.espressif.idf.terminal.connector.serial/META-INF/MANIFEST.MF (1 hunks)
Additional comments (Suppressed): 1
bundles/com.espressif.idf.terminal.connector.serial/META-INF/MANIFEST.MF (1)
  • 18-21: The org.slf4j.api bundle has been removed from the Require-Bundle section, which indicates that the dependency on the SLF4J plugin has been eliminated. This change is in line with the PR summary. However, ensure that all logging calls previously using SLF4J have been updated to use the new logging system (presumably org.eclipse.ui.console). Also, verify that there are no remaining imports or references to org.slf4j in the codebase.

Comment on lines 18 to +21
com.espressif.idf.ui,
com.espressif.idf.serial.monitor,
org.eclipse.cdt.core.native,
org.eclipse.ui.console,
org.slf4j.api
org.eclipse.ui.console
Copy link

Choose a reason for hiding this comment

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

Adding org.eclipse.ui.console as a required bundle suggests a shift from SLF4J to Eclipse's console for logging or output purposes. Ensure that this bundle provides all the necessary functionalities and meets the performance requirements for your logging needs.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between c87d397 and e0efd1b.
Files selected for processing (1)
  • releng/com.espressif.idf.target/com.espressif.idf.target.target (1 hunks)
Files skipped from review due to trivial changes (1)
  • releng/com.espressif.idf.target/com.espressif.idf.target.target

@kolipakakondal
Copy link
Collaborator Author

Hi @AndriiFilippov Would you mind accepting this fix for v2.11.0 release, this would unblock update site issue which we are having with Eclipse 2023-09?

Tests Required:
Take PR update site and test it on Eclipse 2023-09 and Eclipse 2023-06

@kolipakakondal kolipakakondal merged commit 086a97c into master Sep 21, 2023
7 checks passed
@kolipakakondal kolipakakondal deleted the IEP-1025 branch September 21, 2023 11:21
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.

Installation of Espressif IDF Plugin on Eclipse CDT 2023-09 (4.29.0) fails (IEP-1025)
1 participant