-
Notifications
You must be signed in to change notification settings - Fork 0
Update unix configuration #4
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
base: master
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @vojta-horanek, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly updates and expands the Unix configuration for development environments. It focuses on modernizing the Android development stack by upgrading the Android SDK to API 35 and introducing a dedicated Docker image for Android emulators integrated with Maestro for UI automation. Additionally, it brings the Kotlin compiler and Gradle build tool to their latest stable versions, ensuring a robust and up-to-date foundation for building and testing applications. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request updates various Unix configurations, primarily focusing on Docker images for Android development. It introduces a new Dockerfile for an Android emulator with Maestro, updates versions of Kotlin and Gradle, and adds newer Android SDK components. My review includes suggestions to remove temporary debugging commands, improve Dockerfile efficiency by consolidating commands and cleaning up build artifacts, fixing a critical bug in a shell script, and removing dead or redundant code for better maintainability.
| echo "${G_LOG_E} Failed to store x11vnc password." | ||
| exit 1 | ||
| fi | ||
| passwordArgument=-"-rfbauth ${passwordFilePath}" |
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.
There's a critical bug here. The extra hyphen in passwordArgument=-'-rfbauth ...' will cause the argument to be passed incorrectly to x11vnc, which will likely prevent the VNC server from starting correctly with password authentication.
| passwordArgument=-"-rfbauth ${passwordFilePath}" | |
| passwordArgument="-rfbauth ${passwordFilePath}" |
| RUN apt-get update && apt-get install -y \ | ||
| fonts-noto \ | ||
| fonts-noto-mono \ | ||
| fonts-noto-color-emoji && fc-cache -f -v |
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.
This RUN instruction has two issues:
- It runs
apt-get updateagain, which is inefficient. It's better to combine package installations into a singleRUNlayer with oneapt-get updatecall (like the one on line 18). - It's missing
rm -rf /var/lib/apt/lists/*afterapt-get install, which will unnecessarily increase the Docker image size.
I recommend moving the font installation to the RUN block at line 18 and removing this block.
|
|
||
| FROM ${JAVA_IMAGE} | ||
|
|
||
| RUN echo ${JAVA_IMAGE} |
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.
| RUN echo y | sdkmanager "tools" && \ | ||
| echo y | sdkmanager "platform-tools" && \ | ||
| echo y | sdkmanager "build-tools;35.0.0" && \ | ||
| echo y | sdkmanager "build-tools;34.0.0" && \ | ||
| echo y | sdkmanager "build-tools;33.0.2" && \ | ||
| echo y | sdkmanager "build-tools;33.0.1" && \ | ||
| echo y | sdkmanager "build-tools;32.1.0-rc1" && \ | ||
| echo y | sdkmanager "build-tools;30.0.3" |
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.
To improve efficiency and readability, these chained sdkmanager commands can be consolidated into a single call. The sdkmanager tool supports installing multiple packages at once, which reduces the number of processes spawned during the build.
RUN echo y | sdkmanager "tools" "platform-tools" "build-tools;35.0.0" "build-tools;34.0.0" "build-tools;33.0.2" "build-tools;33.0.1" "build-tools;32.1.0-rc1" "build-tools;30.0.3"
| RUN echo y | sdkmanager "platforms;android-35" && \ | ||
| echo y | sdkmanager "platforms;android-34" && \ | ||
| echo y | sdkmanager "platforms;android-33" && \ | ||
| echo y | sdkmanager "platforms;android-32" && \ | ||
| echo y | sdkmanager "platforms;android-31" |
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.
| if [ $? -ne 0 ]; then | ||
| echo "Error launching emulator" | ||
| return 1 | ||
| fi |
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.
| NC='\033[0m' # No Color | ||
|
|
||
|
|
||
| emulator_name=${EMULATOR_NAME} |
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.
| function check_hardware_acceleration() { | ||
| if [[ "$HW_ACCEL_OVERRIDE" != "" ]]; then | ||
| hw_accel_flag="$HW_ACCEL_OVERRIDE" | ||
| else | ||
| if [[ "$OSTYPE" == "darwin"* ]]; then | ||
| # macOS-specific hardware acceleration check | ||
| HW_ACCEL_SUPPORT=$(sysctl -a | grep -E -c '(vmx|svm)') | ||
| else | ||
| # generic Linux hardware acceleration check | ||
| HW_ACCEL_SUPPORT=$(grep -E -c '(vmx|svm)' /proc/cpuinfo) | ||
| fi | ||
|
|
||
| if [[ $HW_ACCEL_SUPPORT == 0 ]]; then | ||
| hw_accel_flag="-accel off" | ||
| else | ||
| hw_accel_flag="-accel on" | ||
| fi | ||
| fi | ||
|
|
||
| echo "$hw_accel_flag" | ||
| } | ||
|
|
||
| # NOT IN USE ATM. | ||
| hw_accel_flag=$(check_hardware_acceleration) |
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.
| if [ $? -ne 0 ]; then | ||
| echo "Error launching emulator" | ||
| return 1 | ||
| fi |
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.
| # Java 17 is default | ||
| ARG VERSION=17 | ||
|
|
||
| RUN echo ${VERSION} |
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.
No description provided.