Skip to content

Conversation

@alirana01
Copy link
Collaborator

@alirana01 alirana01 commented Oct 16, 2025

Description

Package github-copilot plugins with the Espressif-IDE. It requires some additional packages so trying to see if adding it to target definition can bring those dependencies to the end product.

Fixes # (IEP-1594)

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Checklist

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

Summary by CodeRabbit

  • New Features

    • GitHub Copilot integration added to the Eclipse distribution for AI-assisted code completion.
    • Built-in Terminal feature included for in-IDE terminal access.
  • Chores

    • Installation/package configuration updated to enable the new integrations and streamline setup.

@alirana01 alirana01 self-assigned this Oct 16, 2025
@coderabbitai
Copy link

coderabbitai bot commented Oct 16, 2025

Walkthrough

Added org.eclipse.terminal and com.microsoft.copilot installable units to the target definition and added corresponding for both features in the feature.xml; the Copilot InstallableUnit appears twice in the target file.

Changes

Cohort / File(s) Summary
Target definition (InstallableUnit locations)
releng/com.espressif.idf.target/com.espressif.idf.target.target
Added three new <location> blocks of type="InstallableUnit": one exposing org.eclipse.terminal.feature.feature.group version="0.0.0", and two blocks (duplicate entries) referencing repository https://azuredownloads-g3ahgwb5b8bkbxhd.b01.azurefd.net/github-copilot/ for com.microsoft.copilot.eclipse.feature.feature.group version="0.0.0" with attributes includeAllPlatforms="false", includeConfigurePhase="true", includeMode="planner", includeSource="true".
Feature metadata (includes added)
features/com.espressif.idf.feature/feature.xml
Added <includes> entries for com.microsoft.copilot.eclipse.feature version="0.0.0" and org.eclipse.terminal.feature version="0.0.0` (Copilot include appears duplicated).

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant TargetDef as Target Definition
    participant AzureRepo as Azure Repo (github-copilot)
    participant EclipseRepo as Eclipse Update Site
    participant Planner as Target Planner
    participant Eclipse as Eclipse Target

    Note over TargetDef: New InstallableUnit locations added
    AzureRepo->>TargetDef: metadata for com.microsoft.copilot.eclipse.feature (0.0.0)
    EclipseRepo->>TargetDef: metadata for org.eclipse.terminal.feature (0.0.0)
    TargetDef->>Planner: includeMode="planner", includeConfigurePhase="true"
    Planner->>Eclipse: populate target with referenced features
    Note right of Eclipse: Features referenced also added to feature.xml includes
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • sigmaaa
  • kolipakakondal
  • AndriiFilippov

Poem

🐰 I hopped through XML bright,

Two features tucked into night,
Terminal bells and Copilot's tune,
Duplicates danced beneath the moon,
A tiny hop — the target's right.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "IEP-1594: Package github-copilot plugins with the Espressif-IDE" directly aligns with the primary objective stated in the PR summary, which is to add packaging of the github-copilot plugins with the Espressif-IDE. The changeset confirms this with the addition of the com.microsoft.copilot.eclipse.feature to both the target definition and feature.xml. While the changeset also includes the org.eclipse.terminal.feature as a supporting addition, the title appropriately focuses on the primary objective, which is sufficient per the principle that titles don't need to cover every detail. The title is concise, clear, and specific enough that a teammate scanning the history would immediately understand the change is about integrating Copilot plugins.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch IEP-1594

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f02c69a and 966ddd5.

📒 Files selected for processing (2)
  • features/com.espressif.idf.feature/feature.xml (1 hunks)
  • releng/com.espressif.idf.target/com.espressif.idf.target.target (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • features/com.espressif.idf.feature/feature.xml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build_macos
🔇 Additional comments (2)
releng/com.espressif.idf.target/com.espressif.idf.target.target (2)

10-10: LGTM! Terminal feature addition is consistent.

The addition of the terminal feature follows the established pattern for this location block.


115-118: Original review comment contains unsubstantiated concerns—no issues found.

After verification:

  1. No duplicate exists: Only one Copilot InstallableUnit declaration found at line 117, not two as claimed.
  2. Repository URL is official: The Azure CDN URL is the confirmed official update site for GitHub Copilot for Eclipse, referenced in Eclipse Marketplace and GitHub documentation.
  3. Version 0.0.0 is valid: Standard P2 repository practice for resolving to the latest available version.

The code is correct as-is.

Likely an incorrect or invalid review comment.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

Actionable comments posted: 0

🧹 Nitpick comments (1)
releng/com.espressif.idf.target/com.espressif.idf.target.target (1)

116-116: Consider pinning the Copilot plugin version for reproducible builds.

The Copilot feature uses version="0.0.0", which resolves to the latest available version. While this ensures you get the newest features, it can lead to:

  • Non-reproducible builds across different environments or times
  • Unexpected breaking changes when new versions are released
  • Difficulty troubleshooting issues specific to a version

Other dependencies in this file use specific version constraints (e.g., embedcdt at line 78-81 uses version 6.4.0.202307251916).

Consider pinning to a specific version once you've tested and validated a particular release:

-			<unit id="com.microsoft.copilot.eclipse.feature.feature.group" version="0.0.0"/>
+			<unit id="com.microsoft.copilot.eclipse.feature.feature.group" version="[specific.version]"/>

To find the available versions, query the repository metadata or check Microsoft's release notes for the Eclipse plugin.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8bd0164 and 1423e7a.

📒 Files selected for processing (1)
  • releng/com.espressif.idf.target/com.espressif.idf.target.target (3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build_windows
  • GitHub Check: build
🔇 Additional comments (2)
releng/com.espressif.idf.target/com.espressif.idf.target.target (2)

5-5: Document and verify disabling of the p2 configure phase

  • Two existing <location> entries were changed from includeConfigurePhase="true" to false.
  • Disabling the configure phase may skip essential setup steps for core Eclipse and CDT components.
  • Update the PR description with the rationale and test results, and confirm existing plugins initialize correctly.

114-117: ****

The Azure CDN repository URL is the official Microsoft/GitHub distribution channel for Copilot in Eclipse, as confirmed by Microsoft's documentation. The 404 errors from the curl tests are not problematic—p2 repositories do not expose directory listings to arbitrary requests.

The concerns about third-party hosting and security review are unfounded. The version 0.0.0 is the correct Eclipse p2 notation for fetching the latest version, and no additional verification is required.

Likely an incorrect or invalid review comment.

@sigmaaa sigmaaa added this to the v4.0.0 milestone Nov 4, 2025
</url>

<includes id="com.microsoft.copilot.eclipse.feature" version="0.0.0"/>
<includes id="org.eclipse.terminal.feature" version="0.0.0"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

org.eclipse.terminal.feature - how does this help?

@kolipakakondal
Copy link
Collaborator

Hi @alirana01 can you check the error message
Conventional Commits Check / precommit (pull_request)

@kolipakakondal
Copy link
Collaborator

Given our future plans with Eclipse, we can probably avoid this to prevent maintenance issues. If any user needs GitHub Copilot, they can always install the latest or supported version compatible with the Espressif IDE.

@kolipakakondal
Copy link
Collaborator

Hence taking out from the 4.0.0 release milestone

@kolipakakondal kolipakakondal removed this from the v4.0.0 milestone Nov 17, 2025
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.

4 participants