Skip to content

Conversation

@nbmaiti
Copy link

@nbmaiti nbmaiti commented Feb 4, 2026

-> Add devcontainer configuration for development environment setup.
-> Added go debug
-> Updated launch.json for go debug configuration.

 -> Add devcontainer configuration for development environment setup.
 -> Added go debug
 -> Updated launch.json for go debug configuration.

Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
@nbmaiti nbmaiti requested a review from Copilot February 4, 2026 19:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds devcontainer configurations to standardize the development environment for the project. It includes setup for both standard and MEI device development scenarios, along with improved Go debugging capabilities.

Changes:

  • Added two devcontainer configurations: one for standard development without MEI device and one for MEI device development on Linux hosts
  • Enhanced VS Code launch configurations with dynamic input variables for server address, profile name, and DNS suffix
  • Removed asRoot parameter from all Go debug configurations in launch.json

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
.vscode/launch.json Updated Go debug configurations to use input variables and removed deprecated asRoot settings
.devcontainer/standard/devcontainer.json Added standard devcontainer configuration with Go 1.25 and proxy support
.devcontainer/pre-create.sh Added pre-creation script to check/install Intel LMS service on the host
.devcontainer/post-create.sh Added post-creation script for proxy configuration and Go tool installation
.devcontainer/mei/devcontainer.json Added MEI device devcontainer with privileged access and host networking

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +13 to +15
# Remove all trailing / or \
url="${url%%*(/|\\)}"
# Fallback for Bash < 4.0 (no extglob): use sed
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

This extended glob pattern syntax requires shopt -s extglob to be enabled, but the script doesn't set it. The pattern *(/|\\) will not work as intended without extglob. Either add shopt -s extglob at the beginning of the script or rely solely on the sed fallback on line 16.

Suggested change
# Remove all trailing / or \
url="${url%%*(/|\\)}"
# Fallback for Bash < 4.0 (no extglob): use sed
# Remove all trailing / or \ using sed

Copilot uses AI. Check for mistakes.
"runArgs": [
"--device",
"/dev/mei0",
"--privileged",
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

Running the container with --privileged grants extensive permissions that may not all be necessary. Consider using more specific capabilities instead (e.g., --cap-add for required capabilities) to follow the principle of least privilege.

Suggested change
"--privileged",

Copilot uses AI. Check for mistakes.
@graikhel-intel
Copy link
Contributor

Thanks for doing this @nbmaiti!

One suggestion to make this easier to review, it might help to split this into two PRs.

  • One PR for the launch.json changes (dynamic inputs), which looks straightforward and we should be able to merge that quickly.

  • A second PR focused on the devcontainer additions.

For the devcontainer piece specifically, the current config appears to be Linux only. On Windows hosts, VS Code will still prompt users to reopen in a dev container, but the flow itself won’t work for them. This could be confusing for Windows based contributors, especially since many of our developers (and customers) still use Windows. I just want to make sure we’re aligned on the intended developer experience before merging this part.

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.

2 participants