-
Notifications
You must be signed in to change notification settings - Fork 123
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
Update orbit builds, eclipse plugins, jna plugins etc #1111
Conversation
WalkthroughThis pull request involves significant updates to the Eclipse plugin configuration for the Espressif IDF target. The changes primarily focus on updating repository locations from older versions (4.32, 2024-06) to newer versions (4.33, 2024-09). Multiple units and bundles have been removed, including testing frameworks, logging libraries, and XML-related dependencies. New units like Apache Batik and LSP4J have been added, and many unit versions have been reset to Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
releng/com.espressif.idf.target/com.espressif.idf.target.target (1)
41-42
: Review version management strategyUsing version "0.0.0" for multiple dependencies (JNA, TM4E, etc.) can lead to build reproducibility issues.
Consider:
- Using specific versions for critical dependencies
- Documenting the version resolution strategy
- Implementing a version catalog or BOM (Bill of Materials) to manage versions centrally
For example:
- <unit id="com.sun.jna" version="0.0.0"/> + <unit id="com.sun.jna" version="5.13.0"/>Also applies to: 92-93
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
releng/com.espressif.idf.target/com.espressif.idf.target.target
(5 hunks)releng/com.espressif.idf.update/category.xml
(0 hunks)
💤 Files with no reviewable changes (1)
- releng/com.espressif.idf.update/category.xml
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build_macos
- GitHub Check: build
🔇 Additional comments (2)
releng/com.espressif.idf.target/com.espressif.idf.target.target (2)
43-46
: Clarify the requirement for Apache Batik dependenciesThe addition of Apache Batik components suggests new SVG-related functionality. Please clarify:
- The specific use case requiring SVG support
- Which parts of the plugin will use these components
Consider:
- If SVG support is only needed for specific features, consider moving these dependencies to a separate feature to reduce the core plugin's footprint
- Document any SVG-related APIs that will be exposed to other plugins
6-6
: Verify compatibility with Eclipse platform updatesThe updates to Eclipse 4.33 and 2024-09 release are significant platform changes that could impact plugin functionality.
Please ensure:
- All UI components work correctly with Eclipse 4.33
- No regressions in CDT integration
- Plugin activation and extension points are compatible
Run this verification script:
Also applies to: 22-22
✅ Verification successful
Eclipse 4.33 compatibility update appears safe to proceed
The codebase shows minimal risk for the Eclipse 4.33 platform update:
- Limited deprecated API usage (only 4 instances)
- Uses stable, standard Eclipse extension points
- Relies on core Eclipse APIs that maintain backward compatibility
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential compatibility issues with Eclipse 4.33 # Look for deprecated API usage and extension point changes # Search for deprecated API usage rg -l "org\.eclipse\.(ui|core|equinox)" | xargs ast-grep --pattern 'Deprecated|@deprecated' # Check extension point usage rg -A 5 "extension\s+point=\"org\.eclipse" plugin.xmlLength of output: 199
Script:
#!/bin/bash # First, let's find all plugin.xml files and Java source files echo "=== Plugin manifests ===" fd -g "plugin.xml" echo -e "\n=== Checking Java files for deprecated API usage ===" fd -e java | xargs ast-grep --pattern '@Deprecated' echo -e "\n=== Checking extension point declarations ===" fd -g "plugin.xml" -x rg -A 5 'extension\s+point="org\.eclipse' echo -e "\n=== Checking imports of Eclipse packages ===" fd -e java | xargs rg "import\s+org\.eclipse\.(ui|core|equinox)"Length of output: 69209
<repository location="https://download.eclipse.org/lsp4e/snapshots/"/> | ||
<unit id="org.eclipse.lsp4j" version="0.0.0"/> | ||
<unit id="org.eclipse.lsp4j.debug" version="0.0.0"/> | ||
<unit id="org.eclipse.lsp4j.jsonrpc" version="0.0.0"/> | ||
<unit id="org.eclipse.lsp4j.jsonrpc.debug" version="0.0.0"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reconsider using LSP4E snapshot dependencies
Using snapshot versions (download.eclipse.org/lsp4e/snapshots/
) in a production environment can lead to instability and unpredictable behavior.
Consider:
- Using a stable release version instead of snapshots
- If snapshots are necessary, document the specific features requiring them
- Implement version monitoring to track breaking changes in snapshots
- <repository location="https://download.eclipse.org/lsp4e/snapshots/"/>
+ <repository location="https://download.eclipse.org/lsp4e/releases/latest/"/>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<repository location="https://download.eclipse.org/lsp4e/snapshots/"/> | |
<unit id="org.eclipse.lsp4j" version="0.0.0"/> | |
<unit id="org.eclipse.lsp4j.debug" version="0.0.0"/> | |
<unit id="org.eclipse.lsp4j.jsonrpc" version="0.0.0"/> | |
<unit id="org.eclipse.lsp4j.jsonrpc.debug" version="0.0.0"/> | |
<repository location="https://download.eclipse.org/lsp4e/releases/latest/"/> | |
<unit id="org.eclipse.lsp4j" version="0.0.0"/> | |
<unit id="org.eclipse.lsp4j.debug" version="0.0.0"/> | |
<unit id="org.eclipse.lsp4j.jsonrpc" version="0.0.0"/> | |
<unit id="org.eclipse.lsp4j.jsonrpc.debug" version="0.0.0"/> |
Description
Please include a summary of the change and which issue is fixed.
Fixes # (IEP-XXX)
Type of change
Please delete options that are not relevant.
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit
Dependency Updates
Dependency Cleanup
0.0.0
Bundle Removals