Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a development container configuration to streamline the ReVa development environment setup. The changes include a complete Docker-based development environment with automated Ghidra installation and dependency management.
- Adds Docker development container with automated setup for Java 21, Gradle, and Python dependencies
- Updates documentation to reflect current dependency versions (MCP SDK v0.11.2, Jackson 2.19.2, Jetty 11.0.26)
- Enhances installation instructions with new
gradle installcommand option
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.devcontainer/Dockerfile |
Docker container definition with Java 21, Gradle 9.0, and Python environment setup |
.devcontainer/devcontainer.json |
VS Code dev container configuration with extensions and environment variables |
.devcontainer/postCreateCommand.sh |
Automated setup script for cloning/building Ghidra and configuring the environment |
CLAUDE.md |
Updated dependency versions and improved project description |
README.md |
Enhanced installation instructions with automatic installation option |
src/main/java/reva/server/CLAUDE.md |
Updated MCP SDK and Jetty version references |
src/test.slow/java/reva/CLAUDE.md |
Reformatted integration test guidelines with improved structure |
| # Check if GHIDRA_INSTALL_DIR is properly set | ||
| if [ -z "$GHIDRA_INSTALL_DIR" ]; then | ||
| echo "❌ GHIDRA_INSTALL_DIR is not set!" | ||
| exit 1 | ||
| fi | ||
|
|
||
| if [ ! -d "$GHIDRA_INSTALL_DIR" ]; then | ||
| echo "❌ Ghidra directory does not exist at $GHIDRA_INSTALL_DIR" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
The script checks if $GHIDRA_INSTALL_DIR directory exists before Ghidra is actually built and installed. Since Ghidra is built in the previous section (lines 26-41), this check should come after the Ghidra build completion, not before.
| echo "🧪 Running unit tests..." | ||
| if gradle test --info; then | ||
| echo "✅ Unit tests passed!" | ||
| else | ||
| echo "⚠️ Unit tests failed - this might be expected in some environments" |
There was a problem hiding this comment.
[nitpick] The test failure is dismissed as potentially expected without providing specific guidance on when this is acceptable. Consider adding a comment explaining specific scenarios where test failures are expected (e.g., headless environments, missing GUI dependencies) or make the test execution conditional based on environment capabilities.
| echo "🧪 Running unit tests..." | |
| if gradle test --info; then | |
| echo "✅ Unit tests passed!" | |
| else | |
| echo "⚠️ Unit tests failed - this might be expected in some environments" | |
| # In headless environments (e.g., CI, devcontainers without GUI), some tests may fail due to missing GUI dependencies. | |
| # We skip running tests if DISPLAY is not set (Linux) or if running in a known headless environment. | |
| echo "🧪 Running unit tests..." | |
| if [ -z "$DISPLAY" ]; then | |
| echo "⚠️ Skipping unit tests: headless environment detected (no DISPLAY variable set)." | |
| else | |
| if gradle test --info; then | |
| echo "✅ Unit tests passed!" | |
| else | |
| echo "⚠️ Unit tests failed - this might be expected in headless environments or if GUI dependencies are missing." | |
| fi |
No description provided.