-
Notifications
You must be signed in to change notification settings - Fork 13
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
[trixie] Update to Debian Trixie and Python 12 #61
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughThe changes encompass updates across multiple Dockerfiles in the Open Voice OS project, primarily focusing on versioning, formatting, and dependency management improvements. The version number has been incremented to Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Dockerfile
User->>Dockerfile: Request to build image
Dockerfile->>Dockerfile: Set TAG to 0.1.0
Dockerfile->>Dockerfile: Add quotes to variables
Dockerfile->>Dockerfile: Install dependencies with --no-cache-dir
Dockerfile->>User: Image build complete
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: 3
🧹 Outside diff range and nitpick comments (19)
listener/files/requirements.txt (1)
1-1
: Consider adding version constraints for better reproducibility.The inclusion of
ovos-dinkum-listener
with extras, linux, and onnx support is appropriate for the project's needs. However, to ensure reproducibility and avoid potential breaking changes, consider adding version constraints.Consider updating the requirement to include a version constraint:
-ovos-dinkum-listener[extras,linux,onnx] +ovos-dinkum-listener[extras,linux,onnx]~=<current_version>Replace
<current_version>
with the current stable version you're using. This allows for compatible updates while preventing major version jumps that might introduce breaking changes.phal-admin/files/entrypoint.sh (1)
Line range hint
1-18
: Consider additional improvements for robustnessWhile the current changes are good, consider the following improvements to enhance the script's robustness:
- Replace
~
with$HOME
for more reliable home directory expansion across different execution contexts.- Add error handling for the pip install command to catch and report any installation failures.
- Ensure the
~/.local/state/
directory exists before writing the state file.Here's an example of how you might implement these changes:
#!/bin/bash set -e # Exit immediately if a command exits with a non-zero status phal_admin_list="$HOME/.config/mycroft/phal_admin.list" phal_admin_list_state="$HOME/.local/state/phal_admin.state" # Ensure the state directory exists mkdir -p "$(dirname "$phal_admin_list_state")" if [[ -f "$phal_admin_list" ]]; then if ! diff -q -B <(grep -vE '^\s*(#|$)' "$phal_admin_list") <(grep -vE '^\s*(#|$)' "$phal_admin_list_state" 2>/dev/null) &>/dev/null; then if ! pip3 install --no-cache-dir -r "$phal_admin_list"; then echo "Error: Failed to install PHAL admin plugins" >&2 exit 1 fi cp "$phal_admin_list" "$phal_admin_list_state" fi fi # Clear Python cache rm -rf "${HOME}"/.cache # Run ovos_PHAL_admin exec ovos_PHAL_adminThese changes will make the script more resilient to different execution environments and potential errors.
phal/files/entrypoint.sh (2)
5-5
: Approve path change with a suggestion for directory creation.The change from
/tmp/phal.state
to~/local/state/phal.state
is a good improvement for data persistence. However, ensure that the~/local/state/
directory exists before using it.Consider adding a directory creation step before using the new path:
mkdir -p ~/local/state phal_list_state=~/local/state/phal.stateThis will ensure the directory exists regardless of the system configuration.
21-24
: Approve audio server detection order change with a suggestion.The reordering of audio server checks to prioritize PipeWire over PulseAudio is a good update, aligning with modern audio stack preferences while maintaining compatibility.
Consider adding an else clause to handle cases where neither PipeWire nor PulseAudio is detected:
if pw-link --links &>/dev/null; then echo -e 'pcm.!default pipewire\nctl.!default pipewire' >"$asoundrc_file" elif pactl info &>/dev/null; then echo -e 'pcm.!default pulse\nctl.!default pulse' >"$asoundrc_file" else echo "Warning: Neither PipeWire nor PulseAudio detected. Audio might not work correctly." >&2 fiThis addition would provide better error handling and user feedback in unexpected scenarios.
messagebus/Dockerfile (1)
2-2
: LGTM! Minor suggestion for consistency.The use of double quotes around the FROM instruction is a good practice, especially when using variables. For even better consistency across the Dockerfile, consider using double quotes for all string values in ARG and ENV instructions as well.
Here's a suggestion for consistency:
-ARG TAG=0.1.0 +ARG TAG="0.1.0"gui-websocket/Dockerfile (1)
24-29
: LGTM with suggestion: Improved dependency installation and cleanupThe changes to use pip3 explicitly, add the --no-cache-dir option, and remove the requirements.txt file after installation are all good practices. They improve clarity, reduce image size, and ensure proper cleanup.
However, the conditional installation based on the ALPHA variable seems unnecessary as both branches perform the same action. Consider simplifying this to a single pip3 install command.
Here's a suggested simplification:
-RUN if [ "${ALPHA}" == "true" ]; then \ - pip3 install --no-cache-dir -r /tmp/requirements.txt --pre; \ - else \ - pip3 install --no-cache-dir /tmp/requirements.txt; \ - fi \ +RUN pip3 install --no-cache-dir -r /tmp/requirements.txt \ && mkdir "${HOME}/.cache/ovos_gui_file_server" \ && rm -rf "${HOME}/.cache/pip" /tmp/requirements.txtThis change maintains the functionality while simplifying the Dockerfile.
audio/files/entrypoint.sh (2)
6-6
: Approve change with minor suggestionThe modification of
audio_list_state
to use~/.local/state/audio.state
instead of/tmp/audio.state
is a good improvement. It provides better persistence and security by using a user-specific directory instead of a system-wide temporary directory.Consider adding a check to ensure the
~/.local/state
directory exists, and create it if it doesn't. You can add the following lines right after this variable declaration:mkdir -p "$(dirname "$audio_list_state")"
This will ensure the directory structure exists before attempting to use it later in the script.
Line range hint
1-41
: Suggest improvements for error handling and loggingWhile the script functions well, there are opportunities to enhance its robustness and debuggability:
- Add error handling for critical operations. For example:
set -e # Exit immediately if a command exits with a non-zero status # Function for error handling handle_error() { echo "Error occurred in script at line $1" exit 1 } # Set up the error trap trap 'handle_error $LINENO' ERR
- Implement logging for key operations:
log() { echo "[$(date +'%Y-%m-%d %H:%M:%S')] $*" } # Use it like this: log "Installing packages from $file" pip3 install --no-cache-dir -r "$file" || log "Failed to install packages"
- Consider adding a verbose mode for debugging:
VERBOSE=0 while getopts "v" opt; do case $opt in v) VERBOSE=1 ;; esac done # Use it like this: [ $VERBOSE -eq 1 ] && log "Verbose: Checking for sound server"These additions would make the script more robust and easier to debug, which is particularly valuable in a containerized environment where direct access for troubleshooting may be limited.
listener/files/entrypoint.sh (2)
33-36
: Updated audio server detection orderThe change in the order of audio server detection, now checking for PipeWire before PulseAudio, aligns with the trend of modern Linux distributions moving towards PipeWire. This is a good update that will better support systems using PipeWire.
However, it would be beneficial to add a comment explaining the rationale behind this change, especially for systems that might have both PipeWire and PulseAudio installed.
Consider adding a comment like:
# Check for PipeWire first as it's becoming the preferred audio server in modern Linux distributions
Line range hint
1-38
: Enhance script robustness and maintainabilityWhile the script effectively handles its tasks, consider the following improvements to enhance its robustness and maintainability:
- Add error handling and logging for critical operations.
- Check for the existence of required commands (
pip3
,pw-link
,pactl
) before using them.- Consider using functions to organize the code better, especially for the audio server detection logic.
Here's a sample of how you could implement these suggestions:
#!/bin/bash set -e # Exit immediately if a command exits with a non-zero status. log() { echo "[$(date +'%Y-%m-%d %H:%M:%S')] $1" } check_command() { if ! command -v "$1" &> /dev/null; then log "Error: $1 could not be found" exit 1 fi } setup_audio() { asoundrc_file=~/.asoundrc if test -f ~/.config/mycroft/asoundrc; then cp -rfp ~/.config/mycroft/asoundrc "$asoundrc_file" else if pw-link --links &>/dev/null; then log "PipeWire detected, configuring asoundrc" echo -e 'pcm.!default pipewire\nctl.!default pipewire' >"$asoundrc_file" elif pactl info &>/dev/null; then log "PulseAudio detected, configuring asoundrc" echo -e 'pcm.!default pulse\nctl.!default pulse' >"$asoundrc_file" else log "Warning: Neither PipeWire nor PulseAudio detected" fi fi } # Main script starts here log "Starting OVOS listener setup" # Check for required commands check_command pip3 check_command pw-link check_command pactl # ... (rest of your existing script) setup_audio log "Running ovos-dinkum-listener" ovos-dinkum-listenerThis refactored version includes basic logging, error handling, and function organization. It also checks for the existence of required commands before proceeding.
README.md (1)
3-3
: Version update looks good, consider updating changelog.The Open Voice OS version has been correctly updated to 0.1.0, which aligns with the significant changes mentioned in the PR objectives.
Consider updating the project's changelog (if not already done) to reflect the major changes introduced in this version, such as the transition to Debian Trixie and Python 12.
🧰 Tools
🪛 LanguageTool
[style] ~3-~3: Using many exclamation marks might seem excessive (in this case: 5 exclamation marks for a text that’s 782 characters long)
Context: ...-0.1.0-blue)](https://openvoiceos.com/) [![Docker pulls](https://img.shields.io/do...(EN_EXCESSIVE_EXCLAMATION)
core/files/entrypoint.sh (2)
8-8
: Improved pip installation commandAdding
--no-cache-dir
to the pip install command is a good practice in Docker environments. It ensures that the latest package versions are always installed and helps reduce the final image size.Consider adding
--no-compile
to further reduce image size:-pip3 install --no-cache-dir -r "$skills_list" +pip3 install --no-cache-dir --no-compile -r "$skills_list"This prevents the creation of
.pyc
files, which are not typically needed in a Docker environment and can further reduce the image size.
26-26
: Consistent improvement in pip installation commandThe addition of
--no-cache-dir
to the requirements installation command is consistent with the earlier change and brings the same benefits.For consistency with the previous suggestion, consider adding
--no-compile
here as well:-pip3 install --no-cache-dir -r requirements.txt +pip3 install --no-cache-dir --no-compile -r requirements.txtskills/skill-base/Dockerfile (1)
Discrepancy Confirmed between PR Objectives and Dockerfile Content
Upon verification:
- No Dockerfiles are based on Debian Trixie.
- No Dockerfiles install Python 12.
The
skills/skill-base/Dockerfile
does not align with the PR objectives. Please update the base image to Debian Trixie and adjust the Python installation to version 12 to meet the PR's requirements.🔗 Analysis chain
Line range hint
1-38
: Discrepancy between PR objectives and Dockerfile contentWhile the changes made to the
chown
commands are improvements, there's a significant discrepancy between the PR objectives and the content of this Dockerfile:
- The PR objectives mention switching to Debian Trixie, but this Dockerfile is still based on Alpine (
FROM alpine:edge
).- The PR objectives mention updating to Python 12, but this Dockerfile installs Python 3 (
python3
).These discrepancies need to be addressed to ensure the PR meets its stated objectives. Consider updating the base image to Debian Trixie and adjusting the Python installation to version 12 if that's the intended goal of this PR.
To confirm the current state of the repository and other Dockerfiles, you can run the following commands:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any Debian Trixie based Dockerfiles echo "Searching for Debian Trixie based Dockerfiles:" rg --type dockerfile "FROM debian:trixie" # Check for Python 12 installations echo "Searching for Python 12 installations:" rg --type dockerfile "python12" # List all Dockerfiles in the repository echo "Listing all Dockerfiles:" fd DockerfileLength of output: 1370
bus-server/Dockerfile (1)
41-43
: Improved variable quoting and ENV instruction syntax.The changes in these lines improve the Dockerfile:
- Quoting the
$USER
variable in the USER directive is consistent with previous changes and follows best practices.- The ENV instruction now uses the recommended syntax with an equals sign and no spaces.
These changes enhance consistency and adhere to Dockerfile best practices.
For complete consistency, consider using double quotes for the ENV instruction value as well:
-ENV IS_OVOS_CONTAINER="true" +ENV IS_OVOS_CONTAINER="true"This maintains a consistent style throughout the Dockerfile for all variable assignments and string values.
core/Dockerfile (3)
1-1
: Approved: Version tag updated to 0.1.0The change from 'alpha' to a specific version number (0.1.0) indicates a move towards more stable versioning, which is a good practice. This aligns well with the PR objective of updating to Debian Trixie and Python 12.
Consider documenting the versioning strategy (e.g., semantic versioning) in the project's README or CONTRIBUTING guide to ensure consistency in future updates.
29-34
: Approved: Improved package installation and pip commandsThe changes to the package installation and pip commands are beneficial:
- Adding development packages (libfann-dev, build-essential, python3-dev) is necessary for building some Python packages.
- Using --no-cache-dir in pip install commands reduces the image size by not storing the package cache.
- The conditional installation based on ${ALPHA} is maintained, which is good for flexibility.
Consider using the
--no-install-recommends
flag withapt-get install
for the development packages as well, to further minimize the image size.
37-40
: Approved: Comprehensive cleanup commands addedThe addition of cleanup commands is an excellent practice:
- Removing development packages after use significantly reduces the final image size.
- Cleaning up apt cache and removing unnecessary files is a best practice for Docker images.
- The cleanup commands are comprehensive and well-structured.
These changes will result in a more efficient and smaller Docker image.
Consider adding
&& rm -rf /var/lib/apt/lists/*
to the cleanup commands to remove the apt package lists, which can further reduce the image size.listener/Dockerfile (1)
45-49
: LGTM with a minor suggestion: Quoting in USER and WORKDIR instructionsAdding quotes around the ${USER} variable in the USER and WORKDIR instructions is generally a good practice. It prevents issues if the variable contains spaces or special characters. However, in this specific case, it's unlikely that the USER variable would contain spaces.
Consider removing the quotes if you're certain that the USER variable will never contain spaces or special characters. This would slightly simplify the Dockerfile:
-USER "$USER" +USER ${USER} -WORKDIR "/home/${USER}" +WORKDIR /home/${USER}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (33)
- README.md (1 hunks)
- README_MACOS.md (1 hunks)
- README_WINDOWS.md (1 hunks)
- audio/Dockerfile (3 hunks)
- audio/files/entrypoint.sh (3 hunks)
- audio/files/requirements.txt (1 hunks)
- base/Dockerfile (2 hunks)
- bus-server/Dockerfile (2 hunks)
- cli/Dockerfile (2 hunks)
- cli/files/requirements.txt (1 hunks)
- compose/docker-compose.macos.yml (1 hunks)
- compose/docker-compose.windows.yml (1 hunks)
- compose/docker-compose.yml (1 hunks)
- core/Dockerfile (3 hunks)
- core/files/entrypoint.sh (3 hunks)
- core/files/requirements.txt (1 hunks)
- ggwave-plugin/Dockerfile (2 hunks)
- gui-websocket/Dockerfile (2 hunks)
- gui-websocket/files/requirements.txt (1 hunks)
- gui/gui-original/Dockerfile (2 hunks)
- gui/gui-shell/Dockerfile (2 hunks)
- listener-dinkum/files/requirements.txt (0 hunks)
- listener/Dockerfile (3 hunks)
- listener/files/entrypoint.sh (3 hunks)
- listener/files/requirements.txt (1 hunks)
- messagebus/Dockerfile (1 hunks)
- phal-admin/Dockerfile (3 hunks)
- phal-admin/files/entrypoint.sh (1 hunks)
- phal/Dockerfile (3 hunks)
- phal/files/entrypoint.sh (2 hunks)
- phal/files/requirements.txt (1 hunks)
- skills/skill-base/Dockerfile (2 hunks)
- sound-base/Dockerfile (2 hunks)
💤 Files with no reviewable changes (1)
- listener-dinkum/files/requirements.txt
✅ Files skipped from review due to trivial changes (2)
- gui/gui-original/Dockerfile
- gui/gui-shell/Dockerfile
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~3-~3: Using many exclamation marks might seem excessive (in this case: 5 exclamation marks for a text that’s 782 characters long)
Context: ...-0.1.0-blue)](https://openvoiceos.com/) [![Docker pulls](https://img.shields.io/do...(EN_EXCESSIVE_EXCLAMATION)
README_MACOS.md
[grammar] ~3-~3: The operating system from Apple is written “macOS”
Context: ...ith Docker - [Open Voice OS running on Mac OS with Docker](#open-voice-os-running-on-...(MAC_OS)
🔇 Additional comments (105)
listener/files/requirements.txt (1)
2-2
: Verify the necessity of pyalsaaudio and consider adding version constraints.The inclusion of
pyalsaaudio
is appropriate for low-level audio functionality on Linux systems. However, to ensure reproducibility and avoid potential breaking changes, consider adding version constraints.Consider updating the requirement to include a version constraint:
-pyalsaaudio +pyalsaaudio~=<current_version>Replace
<current_version>
with the current stable version you're using. This allows for compatible updates while preventing major version jumps that might introduce breaking changes.Additionally, let's verify if this dependency is necessary for all installations:
If the usage is conditional or limited to specific scenarios, consider moving this dependency to an optional requirements file or adding a comment explaining its purpose.
core/files/requirements.txt (3)
2-2
: Approve the addition of 'plugins' to ovos-core dependency.The change from
ovos-core[lgpl]
toovos-core[lgpl,plugins]
is a good consolidation of dependencies. This modification likely incorporates functionality that was previously provided by separate packages, which aligns with the PR objectives of updating and simplifying the project's dependency structure.
1-3
: Verify functionality after significant dependency changes.The requirements file has undergone substantial changes, with several git URL dependencies removed and functionality likely consolidated into
ovos-core[lgpl,plugins]
. While this simplification is generally positive, it's crucial to ensure that all necessary functionality is still available after these removals.Please run the following commands to check for any missing imports or functionality in the project:
#!/bin/bash # Description: Check for potential missing imports after dependency changes # Search for imports of removed packages echo "Searching for potentially missing imports..." rg -i -t py "from ovos_bidirectional_translation_plugin import|import ovos_bidirectional_translation_plugin" rg -i -t py "from ovos_translate_server_plugin import|import ovos_translate_server_plugin" rg -i -t py "from ovos_utterance_corrections_plugin import|import ovos_utterance_corrections_plugin" rg -i -t py "from ovos_utterance_plugin_cancel import|import ovos_utterance_plugin_cancel" # Search for any TODOs or FIXMEs related to the removed packages echo "Searching for TODOs or FIXMEs related to removed packages..." rg -i "TODO|FIXME" -g "!*.md" | rg -i "translation|translate|utterance|cancel"If any of these searches return results, it may indicate areas of the code that need to be updated to work with the new dependency structure. Please review and update the code accordingly.
3-3
: Approve the addition of ovos-skill-boot-finished package.The addition of
ovos-skill-boot-finished
as a proper package instead of a git URL is a positive change. This simplifies dependency management and likely ensures a more stable version is used.To ensure we're using the latest version, please run the following command:
Compare the output with the version specified in the requirements.txt file (if any) and update if necessary.
phal/files/requirements.txt (1)
1-5
: LGTM! Dependency consolidation looks good.The changes to the requirements file appear to be a positive step towards consolidating dependencies. The new
ovos-PHAL[linux]
package likely incorporates the functionality of the removed individual PHAL components, which should simplify dependency management.However, please consider the following points:
Ensure that all functionalities previously provided by the removed packages (
ovos-PHAL
,ovos-PHAL-plugin-alsa
,ovos-PHAL-plugin-system
, andovos-plugin-manager
) are now covered by the newovos-PHAL[linux]
package.The removal of
wheel
might affect how the project is packaged or distributed. Verify that this removal doesn't negatively impact your build or distribution processes.Update any relevant documentation to reflect these dependency changes, especially if there are instructions or guides that reference the removed packages.
To verify that the new package includes all necessary functionalities, you can run:
This script will help ensure that the new package is properly installed and that there are no lingering references to the old packages in your codebase.
gui-websocket/files/requirements.txt (1)
1-4
: Verify compatibility with Python 12.The PR objectives mention updating to Python 12. Please ensure that all these packages are compatible with Python 12, as some packages may not have been updated to support this version yet.
To verify the Python version compatibility of these packages, you can run the following script:
This script will display the Python version requirements for each package. Please review the output to ensure all packages are compatible with Python 12.
audio/files/requirements.txt (5)
2-2
: LGTM: Package reference updated correctly.The change from a direct GitHub link to the package name
ovos-bidirectional-translation-plugin
is a good practice. It improves maintainability and aligns with standard dependency management practices.
4-4
: LGTM: Package reference maintained correctly.The retention of
ovos-tts-plugin-polly
as a package name instead of a GitHub link is consistent with best practices for dependency management. This change improves maintainability.
Line range hint
1-7
: Overall review summary: Please provide a comprehensive explanation of these changes.The modifications to this
requirements.txt
file appear to be part of a significant update to the project's dependencies. While the changes align with the PR objectives of updating to Debian Trixie and Python 12, a more comprehensive explanation would be beneficial.Could you please provide an overview of how these changes support the transition to Debian Trixie and Python 12? Specifically:
- How do these changes address the incompatibility with Alpine noted in the PR description?
- Are there any potential impacts on the system's functionality, particularly regarding audio processing and machine learning capabilities?
- Are there any additional steps or configurations required as a result of these dependency changes?
A clear understanding of these points would greatly assist in evaluating the full impact of this update.
To get a broader view of the changes across all requirements files:
#!/bin/bash # Description: Compare all requirements.txt files with their previous versions # Test: Show diff of all requirements.txt files git diff HEAD~1 -- '*requirements.txt'
6-6
: LGTM: New audio dependency added. Please clarify its purpose.The addition of
pyalsaaudio
is noted. This package provides Python bindings for the ALSA audio library, which is commonly used in Linux environments. While this addition seems appropriate given the shift to Debian Trixie, could you please clarify the specific need for this package in the context of this update?To ensure this addition doesn't conflict with existing audio implementations, let's check for any other ALSA-related dependencies:
Line range hint
1-7
: Please clarify the rationale behind package removals.Several packages have been removed from the requirements:
- onnxruntime
- ovos-audio-plugin-mpv
- ovos-audio-plugin-simple
- ovos-ocp-news-plugin
- ovos-ocp-youtube-plugin
Could you please explain the reasoning behind these removals? Specifically:
- How does the removal of
onnxruntime
affect the project's machine learning capabilities?- With the removal of
ovos-audio-plugin-mpv
andovos-audio-plugin-simple
, what will handle audio playback now?- Are the functionalities of
ovos-ocp-news-plugin
andovos-ocp-youtube-plugin
no longer required, or are they being handled differently?These changes seem significant, and understanding the motivation behind them would be helpful for reviewers and future maintainers.
Let's check if these removed packages are still referenced elsewhere in the project:
cli/files/requirements.txt (2)
1-4
: Verify the impact of removed dependencies and consider reinstating PyYAML with version pinning.The dependencies 'neon-mana-utils' and 'PyYAML==6.0' have been removed from the requirements. Please ensure that:
- The removal of 'neon-mana-utils' doesn't break any functionality in the project.
- If PyYAML is still needed, consider adding it back with version pinning to avoid potential compatibility issues.
To check if these removed dependencies are still being used in the project, you can run:
#!/bin/bash # Search for imports or usage of neon-mana-utils and PyYAML in the project echo "Checking for neon-mana-utils usage:" rg --type python "from neon_mana_utils|import neon_mana_utils" echo "Checking for PyYAML usage:" rg --type python "from yaml|import yaml"This will help verify if the removed dependencies are still being referenced in the codebase.
3-3
: Verify the necessity of the new dependency and consider version pinning.A new dependency
ovos-docs-viewer
has been added. Please ensure that this addition is necessary for the project and aligns with the PR objectives.Additionally, consider pinning this dependency to a specific commit or tag for better reproducibility, similar to the suggestion for
ovos-cli-client
.To check the usage of this new dependency in the project, you can run:
This will help verify if the new dependency is being used in the codebase.
phal-admin/files/entrypoint.sh (2)
5-5
: Improved state file locationThe change from
/tmp/phal_admin.state
to~/.local/state/phal_admin.state
is a good improvement. This new location adheres better to the XDG Base Directory Specification and provides a more appropriate place for persistent user-specific data. It's less likely to be cleared during system reboots or cleanup processes, ensuring better state preservation between sessions.
8-8
: Pip installation without cacheThe addition of
--no-cache-dir
to the pip install command is a good practice, especially in containerized environments. This ensures that the latest versions of packages are always installed and can help reduce the final image size by not storing the pip cache. However, be aware that this might slightly increase build times and network usage for repeated builds.phal-admin/Dockerfile (5)
2-2
: Approved: Added quotes to FROM instructionAdding quotes around the image reference in the FROM instruction is a good practice. It prevents potential issues with special characters or spaces in the tag, improving the robustness of the Dockerfile.
12-13
: Approved: Added quotes to LABEL instructionsAdding quotes around the variable references in the LABEL instructions for
org.opencontainers.image.version
andorg.opencontainers.image.created
is a good practice. This ensures that the values are properly interpreted, especially if they contain spaces or special characters, improving the reliability of the metadata labeling.
24-24
: Approved: Added quotes to WORKDIR instructionAdding quotes around the path in the WORKDIR instruction is a good practice. This ensures that the path is properly interpreted, especially if it contains spaces or special characters, improving the reliability of setting the working directory.
Line range hint
1-24
: Overall: Improved Dockerfile robustness and version updateThe changes in this Dockerfile primarily focus on improving formatting and version specification. These modifications enhance the overall robustness and readability of the Dockerfile. The most significant change is updating the tag from 'alpha' to '0.1.0', which may have implications for the base image and overall container behavior.
To ensure these changes haven't introduced any unexpected behavior, please conduct thorough testing of the built image. This should include:
- Building the image successfully
- Running a container from the image
- Verifying all expected functionalities work as intended
You can use the following script to perform basic verification:
#!/bin/bash # Build and test the new image # Build the image docker build -t ovos-phal-admin:test . # Run a container from the image container_id=$(docker run -d ovos-phal-admin:test) # Wait for the container to start sleep 10 # Check if the container is running if docker ps | grep -q $container_id; then echo "Container is running successfully." else echo "Error: Container failed to start or exited prematurely." fi # Clean up docker stop $container_id docker rm $container_id echo "Please perform additional functional tests to ensure all expected behaviors are working correctly."
1-1
: Approved: Version update from alpha to 0.1.0The change from 'alpha' to '0.1.0' indicates a move towards a more stable, versioned release. This is a positive step in terms of version management.
Please ensure that the base image with tag '0.1.0' exists and has been thoroughly tested. You can verify this with the following command:
phal/files/entrypoint.sh (1)
8-8
: Approve addition of --no-cache-dir option.The addition of
--no-cache-dir
to the pip install command is a good practice, especially in containerized environments. This ensures that the latest package versions are always installed, avoiding potential issues with outdated cached packages.While this may slightly increase installation time and bandwidth usage, it significantly improves consistency and reproducibility of the environment.
messagebus/Dockerfile (2)
1-1
: LGTM! Significant version update.The change from
alpha
to0.1.0
indicates a move towards a more stable release. This aligns well with the PR objectives of updating to Debian Trixie and Python 12. It also improves version control and reproducibility of the build process.
Line range hint
22-28
: Verify the necessity of ALPHA argument and conditional installation.Given the move to a specific version (0.1.0) in the TAG argument, it might be worth reconsidering whether the ALPHA argument and conditional installation of ovos-messagebus are still necessary. If you're moving away from using pre-release versions, this conditional logic could potentially be simplified.
Please confirm if the ALPHA argument and conditional installation are still required for your build process. If not, consider simplifying this section of the Dockerfile.
gui-websocket/Dockerfile (6)
1-1
: LGTM: Version update from alpha to 0.1.0The change from "alpha" to a specific version number (0.1.0) indicates a move towards a more stable, versioned release. This is a good practice for better traceability and version management.
2-2
: LGTM: Improved FROM instruction formattingAdding quotes around the image reference in the FROM instruction is a good practice. It prevents potential issues with special characters or spaces in the tag, improving the robustness of the Dockerfile.
12-13
: LGTM: Improved LABEL instruction formattingAdding quotes around the ${VERSION} and ${BUILD_DATE} variables in the LABEL instructions is a good practice. This ensures proper handling of values with spaces or special characters, improving the robustness of the Dockerfile.
21-21
: LGTM: Added COPY instruction for requirements.txtThe addition of the COPY instruction to transfer requirements.txt from the files directory to /tmp is a good practice. It allows for better separation of concerns and easier management of dependencies. Using the /tmp directory for temporary storage is appropriate.
33-33
: LGTM: Improved WORKDIR instruction formattingAdding quotes around the path in the WORKDIR instruction is a good practice. It ensures proper handling of paths with spaces or special characters, improving the robustness of the Dockerfile.
Line range hint
1-33
: Overall assessment: Improved Dockerfile robustness and maintainabilityThe changes made to this Dockerfile primarily focus on improving formatting, versioning, and dependency management. These modifications align well with Docker best practices and enhance the overall robustness and maintainability of the image build process.
Key improvements include:
- Moving from an "alpha" tag to a specific version number (0.1.0).
- Better formatting of instructions (adding quotes where necessary).
- Improved dependency management with a separate requirements.txt file.
- Reduced image size through proper cleanup and use of --no-cache-dir option.
These changes collectively contribute to a more stable and maintainable Dockerfile, which is a positive step for the project.
audio/files/entrypoint.sh (2)
20-20
: Approve addition of --no-cache-dir optionThe addition of
--no-cache-dir
to the pip install command is a good practice, especially in containerized environments. This ensures that the latest versions of packages are always installed, preventing potential issues with outdated cached packages.Note that while this improves consistency and can prevent certain caching-related issues, it may slightly increase installation time and network usage. In most cases, the benefits outweigh these minor drawbacks.
33-36
: Approve change in sound server detection orderThe modification to check for PipeWire before PulseAudio is a forward-thinking change. It aligns with the current trend in Linux audio systems, where PipeWire is becoming more prevalent and is designed to eventually replace PulseAudio in many distributions.
This change ensures that if both PipeWire and PulseAudio are present, PipeWire will be preferred. This is generally desirable as PipeWire offers improved performance and compatibility in many scenarios. However, be aware that this might change the behavior on systems where both are present and PulseAudio was previously being used.
listener/files/entrypoint.sh (2)
6-6
: Improved state file locationThe change of
listener_list_state
path from/tmp/listener.state
to/.local/state/listener.state
is a good improvement. This new location is more appropriate for persistent state storage and aligns with the XDG Base Directory Specification.
20-20
: Optimized pip installationThe addition of
--no-cache-dir
to the pip install command is a good practice, especially in containerized environments. This prevents caching of downloaded packages, which can help reduce the final image size and ensure fresh installations. However, note that this might slightly increase installation time for repeated builds.README.md (4)
4-4
: Documentation version update is consistent.The documentation version has been correctly updated to match the new Open Voice OS version (0.1.0), maintaining consistency across the project.
3-6
: README updates accurately reflect significant project changes.The README.md file has been successfully updated to reflect the new versions of Open Voice OS (0.1.0), Debian (Trixie), and Python (3.12). These changes are consistent with the PR objectives and represent a significant update to the project.
Key points:
- The version bump to 0.1.0 suggests notable improvements or new features.
- The move to Debian Trixie (testing) may offer newer packages but requires careful testing.
- Updating to Python 3.12 provides access to the latest language features and improvements.
These changes may have implications for project dependencies and compatibility. Ensure thorough testing is conducted across the entire project to verify stability with these new versions.
🧰 Tools
🪛 LanguageTool
[style] ~3-~3: Using many exclamation marks might seem excessive (in this case: 5 exclamation marks for a text that’s 782 characters long)
Context: ...-0.1.0-blue)](https://openvoiceos.com/) [![Docker pulls](https://img.shields.io/do...(EN_EXCESSIVE_EXCLAMATION)
6-6
: Python version update aligns with PR objectives.The Python version has been correctly updated to 3.12, which aligns with the PR objectives.
As Python 3.12 introduces some changes that might affect compatibility, please ensure that all project dependencies support this version. Run the following script to check for any potential compatibility issues:
#!/bin/bash # Description: Check for potential compatibility issues with Python 3.12 # Test: Search for Python version specifications in requirements files rg --type pip '(python_version|Python::).*3\.1[01]' # Test: Look for any Python version checks in the code rg 'sys\.version_info\[:(major|minor)\].*3\.1[01]' # Test: Check for usage of deprecated functions or modules rg -i '(asyncore|cgi|cgitb|imp|distutils)'
5-5
: Debian version update aligns with PR objectives.The Debian version has been correctly updated to Trixie, which aligns with the PR objectives.
As Debian Trixie (Debian 13) is currently in the testing phase, please ensure that all project dependencies are compatible with this version. Run the following script to check for any potential compatibility issues:
ggwave-plugin/Dockerfile (9)
2-2
: Approve addition of quotes in FROM instruction.Adding quotes around the image reference in the FROM instruction is a good practice. It improves consistency and prevents potential issues with special characters in image names or tags.
9-10
: Approve addition of quotes in LABEL instructions.Adding quotes around ${VERSION} and ${BUILD_DATE} in the LABEL instructions is a good practice. It ensures proper handling of these variables, especially if they contain spaces or special characters.
18-18
: Approve correction of ENV instruction syntax.The addition of the equals sign in the ENV instruction for DEBIAN_FRONTEND is correct. This change ensures proper syntax for environment variable declaration in Dockerfiles.
28-30
: Approve addition of --no-cache-dir to pip install commands.Adding the --no-cache-dir option to pip install commands is a good practice in Dockerfiles. It prevents caching of downloaded packages, which helps reduce the final image size. However, be aware that this may slightly increase build times for subsequent builds.
35-35
: Approve addition of quotes in cleanup command.Adding quotes around ${HOME}/.cache in the cleanup command is a good practice. It ensures proper handling of the HOME variable, especially if it contains spaces or special characters.
37-37
: Approve addition of quotes in USER instruction.Adding quotes around $USER in the USER instruction is a good practice. It ensures proper handling of the USER variable, especially if it contains spaces or special characters.
41-41
: Approve addition of quotes in WORKDIR instruction.Adding quotes around "/home/${USER}" in the WORKDIR instruction is a good practice. It ensures proper handling of the path, especially if the USER variable contains spaces or special characters.
Line range hint
1-41
: Overall improvements in Dockerfile syntax and best practices.The changes made to this Dockerfile represent a series of improvements in syntax, variable handling, and adherence to best practices. These include:
- Updating the TAG to a specific version.
- Adding quotes around variable expansions and paths.
- Correcting the syntax of ENV instructions.
- Adding the --no-cache-dir option to pip install commands.
These changes enhance the robustness and maintainability of the Dockerfile. They align well with the PR objectives of updating versions and improving code quality. The only potential trade-off is a slight increase in build time due to the --no-cache-dir option, but this is generally outweighed by the benefit of reduced image size.
1-1
: Approve TAG update, but verify impact on base image.The change from "alpha" to a specific version "0.1.0" is good for stability. However, ensure that this version of the base image exists and contains all necessary components for the build.
To verify the existence of this image version, run:
✅ Verification successful
TAG update approved. Image version 0.1.0 exists.
The change from "alpha" to the specific version "0.1.0" enhances stability. The specified base image version has been confirmed to exist and is suitable for the build process.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the specified image version exists docker pull smartgic/ovos-sound-base:0.1.0 if [ $? -eq 0 ]; then echo "Image version 0.1.0 exists and was pulled successfully." else echo "Error: Image version 0.1.0 does not exist or couldn't be pulled." fiLength of output: 280
Script:
#!/bin/bash # Check if the specified image tag exists using Docker Hub API REPO="smartgic/ovos-sound-base" TAG="0.1.0" API_URL="https://registry.hub.docker.com/v2/repositories/${REPO}/tags/${TAG}/" response=$(curl -s -o /dev/null -w "%{http_code}" "$API_URL") if [ "$response" -eq 200 ]; then echo "Image version ${TAG} exists." else echo "Image version ${TAG} does not exist." fiLength of output: 367
cli/Dockerfile (7)
2-2
: Approved: Added quotes to FROM instructionThe addition of quotes around the image name improves consistency with other quoted strings in the Dockerfile. This change doesn't affect functionality but enhances readability.
25-27
: Approved: Added --no-cache-dir to pip install commandsThe addition of
--no-cache-dir
to the pip install commands is a good practice for Docker images. It prevents caching of package installations, which can:
- Reduce the final size of the Docker image
- Ensure fresh package installations every time the image is built
Note that this may slightly increase build times for subsequent builds, as packages won't be cached. However, the benefits in terms of image size and consistency usually outweigh this drawback.
29-30
: Approved: Added quotes around ${USER} in mkdir and chown commandsThe addition of quotes around
${USER}
in themkdir
andchown
commands is a good practice. It ensures proper handling of the variable, especially if it contains spaces or special characters. This change improves the robustness of the Dockerfile and prevents potential issues with unusual usernames.
36-36
: Approved: Added quotes to USER instructionThe addition of quotes around
$USER
in the USER instruction is consistent with the changes made in themkdir
andchown
commands. This ensures proper handling of the USER variable and improves the overall robustness of the Dockerfile.
43-43
: Approved: Updated quoting in WORKDIR instructionThe change in the WORKDIR instruction, moving the quotes to encompass the entire path including the
${USER}
variable, is consistent with the quoting style used in other parts of the Dockerfile. This ensures proper handling of the entire path, including the variable, and improves the overall consistency and robustness of the Dockerfile.
Line range hint
1-43
: Overall assessment: Improvements in consistency and best practicesThe changes made to this Dockerfile represent a series of improvements that enhance consistency, robustness, and align with best practices for Dockerfile writing. Key improvements include:
- Updating the version from 'alpha' to a specific version number.
- Consistent use of quotes around variables and paths.
- Addition of the
--no-cache-dir
option to pip install commands.These changes contribute to a more maintainable and reliable Dockerfile, which is in line with the PR objectives of updating to Debian Trixie and Python 12.
1-1
: Approved: Version update from 'alpha' to '0.1.0'This change indicates a move towards a more stable release, which aligns with the PR objectives.
To ensure version consistency across the project, run the following script:
base/Dockerfile (6)
1-1
: Base image update aligns with PR objectives.The change from
debian:bookworm-slim
todebian:trixie-slim
is in line with the PR objective to transition to Debian Trixie. This update forms the foundation for the subsequent Python 12 integration.
26-27
: Improved virtual environment activation method.The change from using
.
tosource
for activating the virtual environment is a good practice. This modification enhances script readability and ensures better compatibility across different shell environments.
29-33
: Improved file ownership syntax and thorough cleanup.The
chown
command now uses the:
separator instead of a space, which is more consistent with modern best practices. The cleanup process remains thorough, ensuring a slim final image by removing unnecessary packages and cleaning up cache and log files.
37-39
: Correct removal of quotes from environment variable values.The removal of quotes from the environment variable values is correct. In Dockerfiles, environment variables don't typically require quotes unless they contain spaces or special characters. This change ensures that the variables are set cleanly without unnecessary string delimiters.
Line range hint
1-39
: Overall, the Dockerfile changes successfully align with PR objectives.The modifications to the Dockerfile effectively transition the base image to Debian Trixie and prepare for Python 12 integration. The changes improve readability, follow best practices, and maintain a clean, efficient build process. The suggested verifications will ensure that all necessary components are in place for the new environment.
18-23
: Environment setup changes look good, but verify package list.The explicit setting of
DEBIAN_FRONTEND=noninteractive
is a good practice for automated builds. The package list has been updated to includepython3-wheel
, which is beneficial for Python package management.Please verify that
python3-minimal
is no longer needed in the new Debian Trixie environment. You can run the following command to check the Python version and ensure it meets the requirements:✅ Verification successful
Package list verification successful;
python3-minimal
is not present.The absence of
python3-minimal
in thebase/Dockerfile
confirms that it is no longer required in the Debian Trixie environment.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
docker build -t temp_image . && docker run --rm temp_image python3 --versionLength of output: 104
Script:
#!/bin/bash # Check if 'python3-minimal' is present in base/Dockerfile grep -i 'python3-minimal' base/DockerfileLength of output: 41
sound-base/Dockerfile (7)
1-1
: LGTM: Version update from alpha to 0.1.0This change indicates a move towards a more stable, versioned release. It's a positive step for better version tracking and potentially improved stability.
2-2
: LGTM: Added quotes to FROM instructionAdding quotes around the image reference in the FROM instruction is a good practice. It ensures consistency and prevents potential issues with special characters in image names or tags.
9-10
: LGTM: Added quotes to LABEL valuesAdding quotes around the values for version and creation date labels is a good practice. It ensures consistency and prevents potential issues with special characters in label values.
30-31
: LGTM: Improved quoting in directory and permission managementAdding double quotes around directory paths and the USER variable in the mkdir and chown commands is a good practice. It improves the robustness of the script by correctly handling potential spaces or special characters in paths and variables.
34-34
: LGTM: Added quotes to USER instructionAdding double quotes around the USER variable in the USER instruction is consistent with the earlier changes and follows best practices. It improves the robustness of the Dockerfile by correctly handling potential spaces or special characters in the user name.
36-36
: LGTM: Simplified ENV instructionRemoving quotes around the boolean value in the ENV instruction for IS_OVOS_CONTAINER is acceptable and improves readability.
Please verify the consistency of ENV instructions throughout the Dockerfile. Run the following script to check for any inconsistencies:
#!/bin/bash # Description: Check consistency of ENV instructions in the Dockerfile # Test: List all ENV instructions in the Dockerfile rg --type dockerfile "^ENV" sound-base/Dockerfile # Test: Check for any ENV instructions with quoted true/false values rg --type dockerfile '^ENV.*=("true"|"false")' sound-base/Dockerfile
24-26
: LGTM: Package management improvementsThe addition of build dependencies (libasound2-dev, build-essential, python3-dev) and their subsequent removal is a good practice for reducing the final image size while ensuring all necessary build tools are available.
Please verify that pyalsaaudio still functions correctly after purging the build dependencies. Run the following script to check for any potential issues:
core/files/entrypoint.sh (3)
5-5
: Improved path for skills state persistenceThe change from
/tmp/skills.state
to~/.local/state/skills.state
is a good improvement. It ensures that the skills state persists across container restarts and follows the XDG Base Directory Specification for user-specific state files.
Line range hint
1-43
: Overall assessment: Improvements in persistence, installation, and sound server handlingThe changes in this file represent improvements in several areas:
- Better persistence of skills state
- Optimized pip installation commands
- Updated sound server detection logic
These modifications enhance the script's functionality and align with Docker best practices. The suggestions for adding
--no-compile
to pip commands and clarifying the sound server priority change will further improve the script's efficiency and maintainability.
40-43
: Revised sound server detection orderThe change in the order of checks, prioritizing PipeWire over PulseAudio, appears to be intentional and maintains the functionality to detect both sound servers.
Could you please clarify the rationale behind prioritizing the PipeWire check? This information would be valuable for future maintenance and understanding of the system requirements.
To verify the prevalence of PipeWire vs PulseAudio in the project, we can run the following script:
This will help us understand if there's a broader shift towards PipeWire in the project that justifies this change in priority.
✅ Verification successful
Sound server detection order verified
The prioritization of PipeWire over PulseAudio aligns with its extensive presence throughout the codebase, ensuring optimal sound server detection based on project dependencies.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for references to PipeWire and PulseAudio in the project echo "PipeWire references:" rg -i pipewire echo "\nPulseAudio references:" rg -i pulseaudioLength of output: 4570
skills/skill-base/Dockerfile (2)
19-19
: Improved syntax for chown commandThe change correctly modifies the user:group specification in the
chown
command. Removing the quotes around the colon is the proper syntax. Additionally, the consistent use of double quotes for path strings is a good practice for shell commands, especially when using variable interpolation.
30-30
: Consistent improvement in chown syntaxThis change mirrors the improvement made on line 19, ensuring consistency throughout the Dockerfile. The user:group specification in the
chown
command is now correctly formatted.bus-server/Dockerfile (3)
7-7
: Improved variable quoting in git clone command.The addition of double quotes around the
$BRANCH
variable in the git clone command is a good practice. This change prevents potential issues with word splitting or globbing, especially if the branch name contains spaces or special characters.
36-36
: Enhanced variable quoting in useradd command.The addition of double quotes around the
$USER
variable in the useradd command is a good practice. This change ensures that the command works correctly even if the username contains spaces or special characters.
Line range hint
7-43
: Overall improvement in Dockerfile robustness and consistency.The changes made to this Dockerfile collectively enhance its robustness and consistency. By properly quoting variables and adjusting the ENV instruction syntax, the file now adheres more closely to best practices for Dockerfile creation. These modifications reduce the risk of potential issues arising from special characters or spaces in variable values, particularly in branch names or usernames.
While the functional impact of these changes may be minimal in many cases, they represent a valuable improvement in code quality and maintainability. Good job on these enhancements!
phal/Dockerfile (8)
2-2
: Approve the addition of quotes in the FROM instruction.Adding quotes around the image reference in the FROM instruction is a good practice. It ensures consistency and prevents potential issues with special characters in the tag.
12-13
: Approve the addition of quotes in LABEL instructions.Adding quotes around ${VERSION} and ${BUILD_DATE} in the LABEL instructions is a good practice. It ensures proper string handling, especially if these variables might contain spaces or special characters.
31-33
: Approve the addition of --no-cache-dir option to pip install commands.Adding the --no-cache-dir option to pip install commands is a good practice in Dockerfiles. It prevents caching of the installed packages, which can help reduce the size of the Docker image. However, be aware that this may slightly increase build times for subsequent builds as packages can't be installed from the cache.
35-35
: Approve the correction in the chown command syntax.The change from
chown "${USER} ${USER}"
tochown "${USER}:${USER}"
is correct. This is the proper syntax for specifying both user and group in a chown command. The colon (:) separates the user and group, while a space would have been interpreted as two separate arguments.
39-39
: Approve the addition of quotes in the cleanup command.Adding quotes around ${HOME} in the cleanup command is a good practice. It ensures proper handling of the path, especially if it might contain spaces or special characters.
45-45
: Approve the addition of quotes in the WORKDIR instruction.Adding quotes around the path in the WORKDIR instruction is a good practice. It ensures proper handling of the path, especially if it might contain spaces or special characters.
Line range hint
1-45
: Overall approval: Dockerfile improvements enhance robustness and consistency.The changes made to this Dockerfile are all positive improvements:
- Updated versioning from "alpha" to "0.1.0"
- Added quotes to various instructions for better string handling
- Improved pip install commands with --no-cache-dir option
- Corrected chown command syntax
- Enhanced cleanup and WORKDIR instructions
These changes align with the PR objectives and improve the overall quality and robustness of the Dockerfile. Good job on these updates!
1-1
: Approve version update and suggest consistency check.The TAG has been updated from "alpha" to "0.1.0", indicating a move to a more stable release. This aligns with the PR objective of updating to Debian Trixie and Python 12.
To ensure version consistency across the project, run the following script:
✅ Verification successful
Version consistency verified across the project.
The TAG has been updated to "0.1.0" consistently across all relevant files, fulfilling the suggested consistency check.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for version consistency across the project # Test: Search for version numbers in Dockerfiles and other relevant files rg --type-add 'docker:*.{dockerfile,Dockerfile}' --type docker --type yaml --type md '0\.1\.0'Length of output: 682
audio/Dockerfile (8)
1-1
: LGTM: Version update from alpha to 0.1.0This change from an alpha tag to a specific version number (0.1.0) is a positive step. It improves version tracking and reproducibility of the build process.
2-2
: LGTM: Improved FROM instruction formattingThe addition of quotes around the image name in the FROM instruction is a good practice. It ensures consistency and prevents potential issues with special characters in image names.
21-21
: LGTM: Improved ENV variable syntaxThe addition of the equal sign in the ENV instruction follows the standard Docker syntax for setting environment variables. This change improves readability and maintains consistency with best practices.
33-33
: LGTM: Added --no-cache-dir to pip install commandsThe addition of the --no-cache-dir option to pip install commands is a good practice for Docker builds. It helps reduce the final image size by preventing the storage of unnecessary cache files.
Note: This may slightly increase build times for subsequent builds as packages can't be retrieved from cache. However, the trade-off is generally worth it for reduced image size and cleaner builds.
Also applies to: 35-35
37-37
: LGTM: Improved mkdir command with quotesThe addition of double quotes around the path in the mkdir command is a good practice. It ensures that the command works correctly even if the USER variable contains spaces or special characters, improving the robustness of the script.
38-38
: LGTM: Corrected chown command syntaxThe change from a space to a colon in the chown command to separate user and group is correct. This follows the standard chown syntax and ensures proper ownership setting for the user's home directory.
42-42
: LGTM: Improved cleanup and variable usage
- The cleanup command now removes the entire .cache directory, which is more thorough and ensures a cleaner final image.
- The addition of double quotes around the $USER variable in the USER and WORKDIR instructions is a good practice. It ensures correct handling of the variable even if it contains spaces or special characters.
These changes contribute to a more robust and cleaner Dockerfile.
Also applies to: 44-44, 48-48
Line range hint
1-48
: Overall: Excellent improvements to the DockerfileThe changes made to this Dockerfile demonstrate a commitment to best practices and improved consistency. Key improvements include:
- Moving from an alpha tag to a specific version number.
- Better formatting and quoting in various instructions.
- Improved package installation process with the --no-cache-dir option.
- More thorough cleanup process.
- Consistent use of quotes around variables to handle potential special characters.
These changes contribute to a more robust, maintainable, and efficient Dockerfile. Great job on the improvements!
core/Dockerfile (5)
2-2
: Approved: Improved FROM instruction formattingAdding quotes around the image name in the FROM instruction is a good practice. It ensures consistency and prevents potential issues with special characters in image names or tags.
12-13
: Approved: Improved LABEL instruction formattingThe addition of quotes around ${VERSION} and ${BUILD_DATE} in the LABEL instructions is a good practice. It improves readability and prevents potential issues with spaces or special characters in the variable values.
35-36
: Approved: Improved directory and ownership command formattingThe addition of quotes around paths and variables in the directory creation and ownership change commands is a good practice. It improves the robustness of the script by correctly handling potential spaces or special characters in paths or usernames.
42-42
: Approved: Improved USER and WORKDIR instruction formattingThe addition of quotes around ${USER} in the USER and WORKDIR instructions is a good practice. It improves readability and prevents potential issues with spaces or special characters in usernames.
Also applies to: 46-46
Line range hint
1-46
: Overall assessment: Significant improvements to the DockerfileThe changes made to this Dockerfile represent a substantial improvement in several areas:
- Updated versioning strategy, moving from 'alpha' to a specific version number.
- Improved formatting and consistency throughout the file, including proper quoting of variables and paths.
- Enhanced package management with the addition of necessary development packages and subsequent removal.
- Implementation of best practices for Docker image creation, such as using --no-cache-dir for pip and comprehensive cleanup commands.
These changes align well with the PR objectives of updating to Debian Trixie and Python 12, while also resulting in a more efficient, smaller, and more maintainable Docker image. The improvements in formatting and best practices will make future updates easier and reduce the likelihood of errors.
listener/Dockerfile (7)
1-1
: LGTM: Version update from alpha to 0.1.0The change from 'alpha' to a specific version '0.1.0' is a good practice. It improves reproducibility and indicates a move towards a more stable release.
2-2
: LGTM: Added quotes to FROM instructionAdding quotes around the image reference in the FROM instruction is a good practice. It improves readability and helps prevent potential issues with special characters.
12-13
: LGTM: Added quotes to LABEL valuesAdding quotes around the ${VERSION} and ${BUILD_DATE} variables in the LABEL instructions is a good practice. It ensures that the values are treated as single strings, even if they contain spaces or special characters.
21-21
: LGTM: Explicit setting of DEBIAN_FRONTENDThe explicit setting of DEBIAN_FRONTEND to 'noninteractive' using the preferred syntax (with an equal sign) is a good practice. It helps prevent interactive prompts during package installation, which is crucial for automated builds.
32-36
: LGTM: Added --no-cache-dir to pip install commandsAdding the --no-cache-dir option to pip install commands is a good practice in Dockerfiles. It prevents caching of the installed packages, which helps reduce the final image size. However, be aware that this may slightly increase build times as packages can't be reused from cache in subsequent builds.
39-39
: LGTM: Corrected chown syntaxThe change from space to colon in the chown command (${USER}:${USER}) is correct. This is the proper syntax for specifying user:group in chown commands.
43-43
: LGTM: Improved cleanup commandThe modified cleanup command ensures proper syntax for removing directories and includes more comprehensive cleanup. This is a good practice as it helps reduce the final image size by removing unnecessary files and directories.
compose/docker-compose.windows.yml (1)
103-103
: Verify compatibility with the updated ovos-listener image.The image for the
ovos_listener
service has been updated fromdocker.io/smartgic/ovos-listener-dinkum:${VERSION}
todocker.io/smartgic/ovos-listener:${VERSION}
. This change aligns with the PR objectives to update the system, potentially moving away from a specific variant (dinkum) to a more general or updated version of the listener.While this update is likely part of the broader transition to Debian Trixie and Python 12, it's important to ensure that:
- The new image is compatible with the rest of the system.
- There are no unintended side effects or breaking changes introduced by this update.
- The functionality of the listener service remains intact.
To verify the compatibility and functionality of the new image, you can run the following script:
This script will attempt to pull the new image and display its details if successful. It will also check for any version-specific information that might be relevant to this update.
Additionally, please ensure that comprehensive testing is performed to validate the listener's functionality with this new image.
compose/docker-compose.yml (2)
Line range hint
1-283
: Overall file structure and consistencyThe Docker Compose file is well-structured and follows best practices. It defines various services for the Open Voice OS project with appropriate volume mappings, network configurations, and dependencies. The use of YAML anchors for common configurations (like logging and Podman-specific settings) promotes reusability and maintainability.
While only one change was made in this file, it's crucial to ensure that this update aligns with the overall project objectives of transitioning to Debian Trixie and Python 12. The consistency of image versions and configurations across all services should be double-checked to maintain system integrity.
109-109
: Verify the impact of the updated ovos-listener image.The image for the
ovos_listener
service has been changed fromdocker.io/smartgic/ovos-listener-dinkum:${VERSION}
todocker.io/smartgic/ovos-listener:${VERSION}
. This change removes the "-dinkum" suffix, potentially indicating a shift to a more generic version of the listener.Please ensure the following:
- Verify that this new image is compatible with the updated Debian Trixie and Python 12 environment.
- Check if this change affects any functionality or features of the listener service.
- Confirm that this change is consistent with the project's current direction and other services.
- Update any relevant documentation that may reference the old image name.
To help verify the consistency of this change across the project, you can run the following script:
This will help identify any inconsistencies or outdated references in the project.
✅ Verification successful
All references to 'ovos-listener-dinkum' have been successfully removed.
No remaining references to 'ovos-listener-dinkum' were found in the project. All ovos-related images in Docker Compose files are now using the updated naming convention, ensuring consistency across the project.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to 'ovos-listener-dinkum' in the project rg 'ovos-listener-dinkum' --type yaml --type markdown # List all ovos-related images used in Docker Compose files rg 'image:\s+docker\.io/smartgic/ovos-' --type yaml -o | sort | uniqLength of output: 582
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
🧹 Outside diff range and nitpick comments (1)
gui-websocket/Dockerfile (1)
26-32
: LGTM with a minor suggestion: Improved dependency installation and cleanup.The changes to the RUN instruction are good improvements:
- Using pip3 explicitly is clearer.
- Installing from requirements.txt is a best practice.
- The --no-cache-dir option helps reduce image size.
- Cleanup steps are important for reducing image size and improving security.
- The chown command ensures correct permissions.
Consider combining the rm commands to reduce the number of layers:
- && rm -rf "${HOME}/.cache/pip" /tmp/requirements.txt + && rm -rf "${HOME}/.cache/pip" /tmp/requirements.txt /root/.cacheThis will also remove the root user's cache, further reducing the image size.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- ggwave-plugin/Dockerfile (2 hunks)
- gui-websocket/Dockerfile (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ggwave-plugin/Dockerfile
🔇 Additional comments (6)
gui-websocket/Dockerfile (6)
1-1
: LGTM: Specific version tag improves traceability.Changing the TAG from 'alpha' to '0.1.0' is a good practice. It provides better traceability and reproducibility for the image builds.
2-2
: LGTM: Improved syntax for base image reference.Adding quotes around the base image reference in the FROM instruction is a good practice. It ensures consistency and prevents potential issues with special characters in image names or tags.
12-13
: LGTM: Improved robustness in LABEL instructions.Adding quotes around the ${VERSION} and ${BUILD_DATE} variables in the LABEL instructions is a good practice. It ensures that the values are correctly handled even if they contain spaces or special characters.
23-24
: LGTM: Improved dependency management.Adding a separate COPY instruction for the requirements.txt file is a good practice. It allows for better separation of concerns and easier maintenance of the dependencies list.
38-38
: LGTM: Improved WORKDIR instruction syntax.Adding quotes around the path in the WORKDIR instruction is a good practice. It ensures consistency and prevents potential issues with paths containing spaces.
Line range hint
1-38
: Overall, good improvements to the Dockerfile with a minor optimization opportunity.The changes made to this Dockerfile are positive, improving versioning, dependency management, and following best practices. Key improvements include:
- Specific version tagging
- Consistent use of quotes
- Improved dependency management with requirements.txt
- Proper user switching for installations
- Cleanup steps to reduce image size
Consider the suggested optimization to the cleanup process to further reduce the image size. Great work on improving the overall quality and maintainability of the Dockerfile!
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: 5
🧹 Outside diff range and nitpick comments (24)
skills/skill-easter-eggs/Dockerfile (3)
1-1
: Approve the version update and suggest parameterization.The change from 'alpha' to a specific version '0.1.0' is a good move towards better version management. However, to make future updates easier, consider parameterizing this version.
You could modify this line to use an environment variable:
-ARG TAG=0.1.0 +ARG TAG=${OVOS_VERSION:-0.1.0}This allows easy version updates through environment variables while maintaining a default value.
9-10
: Approve the use of quotes in LABEL instructions and suggest consistency.The addition of quotes around the variable substitutions in LABEL instructions is a good practice. It ensures consistency and prevents potential issues with special characters.
For consistency, consider applying the same change to other labels in the file. For example:
-LABEL org.opencontainers.image.title="Open Voice OS OCI easter eggs image" +LABEL org.opencontainers.image.title="${TITLE:-Open Voice OS OCI easter eggs image}"This approach allows for easier customization of labels through environment variables while maintaining default values.
Line range hint
15-21
: Approve the conditional installation logic and suggest optimization.The addition of conditional logic for installing either stable or pre-release versions of the package provides good flexibility for development and testing purposes. The cleanup command to remove the cache is also correctly retained.
To further optimize the installation process and reduce the image size, consider adding the
--no-cache-dir
flag to the pip install commands. This prevents caching of the downloaded packages, which is unnecessary in a Docker context. Here's the suggested change:RUN if [ "${ALPHA}" == "true" ]; then \ - pip3 install skill-easter-eggs --pre; \ + pip3 install --no-cache-dir skill-easter-eggs --pre; \ else \ - pip3 install skill-easter-eggs; \ + pip3 install --no-cache-dir skill-easter-eggs; \ fi \ && rm -rf "${HOME}/.cache"This change will slightly reduce the image size and speed up the build process.
skills/skill-tunein/Dockerfile (2)
1-2
: Approved: Version update and syntax improvementThe changes to the TAG argument and FROM instruction are good improvements:
- Moving from 'alpha' to a specific version ('0.1.0') indicates a shift towards a more stable base image.
- Adding quotes around the image reference in the FROM instruction is a good practice for consistency.
Consider using the
$TAG
variable consistently in double quotes:-FROM "smartgic/ovos-skill-base:${TAG}" +FROM "smartgic/ovos-skill-base:$TAG"This change aligns with shell scripting best practices for variable expansion within double quotes.
18-21
: Approved: Simplified package installationThe change from installing directly from GitHub to using the package name 'ovos-skill-tunein' is a good improvement:
- It simplifies the installation process.
- It suggests the package is now available in a standard package repository (likely PyPI), which is more maintainable.
- The '--pre' flag is still used for alpha builds, maintaining the ability to install pre-release versions when needed.
Consider adding a comment to explain the purpose of the ALPHA flag:
ARG ALPHA=false +# Install the latest stable version or pre-release version based on ALPHA flag RUN if [ "${ALPHA}" == "true" ]; then \ pip3 install ovos-skill-tunein --pre; \ else \ pip3 install ovos-skill-tunein; \ fi \ && rm -rf "${HOME}/.cache"
This addition would improve the readability and maintainability of the Dockerfile.
skills/skill-parrot/Dockerfile (1)
2-2
: Consistent use of quotes approved, with a minor suggestion.The addition of quotes around the image reference is good for consistency. However, to fully leverage this practice:
Consider also quoting the
${TAG}
variable:-FROM "smartgic/ovos-skill-base:${TAG}" +FROM "smartgic/ovos-skill-base:${TAG}"This ensures consistent treatment of all variables and strings throughout the Dockerfile.
skills/skill-alerts/Dockerfile (1)
2-2
: Good addition of quotes, but they're optional here.Adding quotes around the image reference in the FROM instruction is a good practice for consistency with other Dockerfile instructions. However, it's worth noting that quotes are not strictly necessary for the FROM instruction in Dockerfiles.
While this change is fine, you could consider removing the quotes if you prefer a more concise syntax:
FROM smartgic/ovos-skill-base:${TAG}
skills/skill-ggwave/Dockerfile (3)
1-1
: Approved: Specific version tag improves reproducibility.Changing from 'alpha' to a specific version '0.1.0' is good for reproducibility. However, consider using an environment variable for the tag to allow easier updates:
-ARG TAG=0.1.0 +ARG TAG=${OVOS_VERSION:-0.1.0}This way, you can override the version during build time if needed.
18-18
: Approved: Simplified package installation.The change from installing directly from GitHub to using
pip install ovos-skill-ggwave
simplifies the installation process. This is good if the package is available on PyPI or a configured package index.Consider adding a comment explaining the use of the
--pre
flag:- pip3 install ovos-skill-ggwave --pre; \ + # Install pre-release versions when ALPHA is true + pip3 install ovos-skill-ggwave --pre; \This helps clarify the intention behind using the
--pre
flag.
Line range hint
1-24
: LGTM: Overall Dockerfile structure is sound.The overall structure of the Dockerfile, including the unchanged parts, follows good practices:
- The cleanup command helps keep the image size small.
- The ENTRYPOINT correctly uses the skill launcher.
Consider adding a .dockerignore file if not already present to further optimize the build process.
skills/skill-wolfie/Dockerfile (1)
Line range hint
1-24
: Overall changes align with PR objectives, but consider explicit version specificationsThe changes in this Dockerfile align well with the PR objective of updating the project. The move from 'alpha' to a specific version number and the simplification of installation commands contribute to improved stability and maintainability.
However, to fully meet the PR objectives, consider the following suggestion:
Add explicit specifications for Debian Trixie and Python 12 versions in the Dockerfile. This could be done by modifying the base image tag or adding labels. For example:
ARG TAG=0.1.0 FROM "smartgic/ovos-skill-base:${TAG}-debian-trixie-python12" # ... (existing content) ... LABEL org.opencontainers.image.base.name="debian:trixie-slim" LABEL org.opencontainers.image.base.version="12" LABEL org.opencontainers.image.python.version="3.12"This will make the Debian and Python versions explicit and easily verifiable.
skills/skill-persona/Dockerfile (2)
2-2
: LGTM: Improved robustness with quoted image referenceAdding quotes around the image reference is a good practice. It prevents potential issues with special characters or spaces in the TAG value.
Consider using the ARG instruction again just before the FROM instruction to ensure the TAG value is available in this build stage:
ARG TAG FROM "smartgic/ovos-skill-base:${TAG}"This ensures that the TAG value is explicitly available in this build stage, following best practices for multi-stage builds.
Line range hint
17-22
: Consider adding --no-cache-dir to pip install commandsWhile the conditional installation based on the ALPHA flag is good, consider adding the
--no-cache-dir
option to the pip install commands. This can help reduce the final image size by preventing the storage of package cache.Here's a suggested modification:
RUN if [ "${ALPHA}" == "true" ]; then \ pip3 install --no-cache-dir git+https://github.com/OpenVoiceOS/ovos-persona.git --pre; \ else \ pip3 install --no-cache-dir ovos-persona; \ fi \ && rm -rf "${HOME}/.cache"This change will ensure that pip doesn't store the package cache, potentially reducing the final image size.
skills/skill-homescreen/Dockerfile (1)
Line range hint
17-22
: Suggestion: Optimize pip installation process.While the installation logic remains functional, consider optimizing it to reduce the Docker image size:
Apply this diff to optimize the installation process:
RUN if [ "${ALPHA}" == "true" ]; then \ - pip3 install ovos-skill-homescreen --pre; \ + pip3 install --no-cache-dir ovos-skill-homescreen --pre; \ else \ - pip3 install ovos-skill-homescreen; \ + pip3 install --no-cache-dir ovos-skill-homescreen; \ fi \ && rm -rf "${HOME}/.cache"This change adds the
--no-cache-dir
option topip3 install
, which prevents caching of the downloaded packages, reducing the overall size of the Docker image.skills/skill-fallback-unknown/Dockerfile (2)
1-1
: Approved: Version update improves stability.Changing from
alpha
to a specific version0.1.0
is a good practice for reproducibility. However, consider making this version configurable for easier updates.You could modify this line to:
ARG TAG=0.1.0
This allows overriding the version at build time if needed.
Line range hint
1-24
: Consider adding apt cache cleanup.While the Dockerfile looks good overall, consider adding an apt cache cleanup step to reduce the final image size. This is a common best practice in Dockerfile creation.
You could add the following line after the installation step:
RUN apt-get clean && rm -rf /var/lib/apt/lists/*
This will remove the apt cache and reduce the size of the final image.
skills/skill-hello-world/Dockerfile (3)
1-1
: Approved: Specific version tag improves reproducibility.Changing from 'alpha' to a specific version '0.1.0' is good for reproducibility. However, consider implementing a strategy for easy updates in the future, such as using build arguments or environment variables to override this default when needed.
Line range hint
15-21
: Consider reviewing the necessity of the ALPHA argument.Given that we're now using a specific version tag (0.1.0) instead of 'alpha', it might be worth reviewing whether the ALPHA argument and the conditional installation logic are still necessary. If this logic is no longer needed, consider simplifying the installation process.
Line range hint
15-21
: Recommendation: Add --no-cache-dir to pip install command.To reduce the Docker image size and align with best practices, consider adding the --no-cache-dir option to the pip install command. This prevents pip from saving downloaded packages locally, which is unnecessary in a Docker context.
Here's a suggested modification:
RUN if [ "${ALPHA}" == "true" ]; then \ - pip3 install ovos-skill-hello-world --pre; \ + pip3 install --no-cache-dir ovos-skill-hello-world --pre; \ else \ - pip3 install ovos-skill-hello-world; \ + pip3 install --no-cache-dir ovos-skill-hello-world; \ fi \ && rm -rf "${HOME}/.cache"skills/skill-randomness/Dockerfile (1)
2-2
: Approve with suggestion: Maintain consistency in quote usageAdding quotes around the image reference is a good practice. However, for consistency, consider using single quotes here to match the ARG instruction above:
-FROM "smartgic/ovos-skill-base:${TAG}" +FROM 'smartgic/ovos-skill-base:${TAG}'This ensures a consistent style throughout the Dockerfile.
skills/skill-wikipedia/Dockerfile (2)
1-1
: Approved: Specific version tag improves reproducibility.Changing from 'alpha' to a specific version ('0.1.0') is good for reproducibility. However, consider using a variable for the version number to make future updates easier.
You could use an ARG for the version like this:
ARG VERSION=0.1.0 ARG TAG=${VERSION}This way, you can update the version in one place for the entire Dockerfile.
18-18
: Approved with suggestions: Simplified installation, but consider version control.The change to install 'ovos-skill-wikipedia' directly instead of from a GitHub repository simplifies the process and is generally more stable. However, there are a few points to consider:
- The '--pre' flag is still used, which installs pre-release versions. Is this intentional for all builds?
- The ALPHA flag logic has been removed, reducing flexibility in choosing between stable and pre-release versions.
Consider the following adjustments:
If you always want the latest stable version:
RUN pip3 install ovos-skill-wikipedia && rm -rf "${HOME}/.cache"If you want to keep the option to install pre-release versions:
ARG PRERELEASE=false RUN pip3 install ovos-skill-wikipedia$([ "$PRERELEASE" = "true" ] && echo " --pre") && rm -rf "${HOME}/.cache"This way, you can control whether to install pre-release versions using a build argument.
skills/skill-date-time/Dockerfile (2)
1-1
: Approve the version update, but consider flexibility.The change from
alpha
to a specific version0.1.0
is good for reproducibility. However, consider using a build argument to allow for flexibility in specifying the version during build time.You could modify this line to:
ARG TAG=0.1.0
This way, you can override the version during build time if needed, while still having a default value.
Line range hint
18-23
: Optimize pip install command to reduce image size.Consider adding the
--no-cache-dir
option to the pip install commands. This will prevent pip from caching the downloaded packages, which can help reduce the final image size.Update the pip install commands as follows:
RUN if [ "${ALPHA}" == "true" ]; then \ pip3 install --no-cache-dir ovos-skill-date-time ovos-audio --pre; \ else \ pip3 install --no-cache-dir ovos-skill-date-time ovos-audio; \ fi \ && rm -rf "${HOME}/.cache"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (21)
- messagebus/Dockerfile (2 hunks)
- skills/skill-alerts/Dockerfile (1 hunks)
- skills/skill-base/Dockerfile (2 hunks)
- skills/skill-date-time/Dockerfile (1 hunks)
- skills/skill-duckduckgo/Dockerfile (1 hunks)
- skills/skill-easter-eggs/Dockerfile (1 hunks)
- skills/skill-fallback-chatgpt/Dockerfile (2 hunks)
- skills/skill-fallback-unknown/Dockerfile (1 hunks)
- skills/skill-ggwave/Dockerfile (1 hunks)
- skills/skill-hello-world/Dockerfile (1 hunks)
- skills/skill-homescreen/Dockerfile (1 hunks)
- skills/skill-jokes/Dockerfile (1 hunks)
- skills/skill-parrot/Dockerfile (1 hunks)
- skills/skill-persona/Dockerfile (1 hunks)
- skills/skill-personal/Dockerfile (1 hunks)
- skills/skill-randomness/Dockerfile (2 hunks)
- skills/skill-tunein/Dockerfile (1 hunks)
- skills/skill-volume/Dockerfile (1 hunks)
- skills/skill-weather/Dockerfile (2 hunks)
- skills/skill-wikipedia/Dockerfile (1 hunks)
- skills/skill-wolfie/Dockerfile (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- messagebus/Dockerfile
- skills/skill-base/Dockerfile
🔇 Additional comments (64)
skills/skill-duckduckgo/Dockerfile (6)
2-2
: Approve addition of quotes in FROM instructionAdding quotes around the base image reference
"smartgic/ovos-skill-base:${TAG}"
is a good practice. It prevents potential issues with word splitting and globbing, making the Dockerfile more robust.
9-10
: Approve addition of quotes in LABEL instructionsThe addition of quotes around
${VERSION}
and${BUILD_DATE}
in the LABEL instructions is consistent with the quoting style used in the FROM instruction. This change improves the overall robustness of the Dockerfile.
Line range hint
1-24
: Summary: Approve changes with suggested verificationsThe changes in this Dockerfile align well with the PR objectives and represent improvements in versioning, consistency, and simplification. Key points:
- Moving to a specific version (0.1.0) from 'alpha'.
- Improved quoting for robustness.
- Simplified package installation process.
To ensure these changes are consistent across the project, please run the verification scripts provided in the previous comments. These will help confirm:
- TAG version consistency across Dockerfiles.
- Availability and version of the skill-ddg package on PyPI.
- Consistent use of the new package name (skill-ddg) across the project.
After these verifications, the changes can be confidently merged.
20-20
: Approve package name alignment and verify naming consistencyThe change from
ovos-skill-ddg
toskill-ddg
aligns the package name with the one used in the ALPHA=true case. This suggests a possible renaming of the package.To ensure naming consistency across the project, please run:
#!/bin/bash # Description: Verify skill-ddg package naming consistency # Test: Search for any remaining references to 'ovos-skill-ddg' rg 'ovos-skill-ddg'
18-18
: Approve simplified installation command and verify package availabilityThe change from installing directly from GitHub to using
pip3 install skill-ddg --pre
simplifies the installation process. This suggests that the skill-ddg package is now available in a package index (likely PyPI).To ensure the package is available and to check its latest version, please run:
#!/bin/bash # Description: Verify skill-ddg package availability and version # Test: Check if skill-ddg is available on PyPI and get its latest version pip3 install skill-ddg --pre --dry-run
1-1
: Approve TAG update and verify version consistencyThe change from
alpha
to a specific version0.1.0
is a good practice, indicating a move towards a more stable release. This aligns with the PR objective of updating to Debian Trixie and Python 12.To ensure consistency across the project, please run the following script:
✅ Verification successful
TAG version is consistently set to
0.1.0
across all Dockerfiles.All Dockerfiles have been verified to use the updated TAG version, ensuring uniformity and stability aligned with the PR objectives.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify TAG version consistency across Dockerfiles # Test: Search for inconsistent TAG versions rg --type dockerfile 'ARG TAG=' | sort | uniq -cLength of output: 112
Script:
#!/bin/bash # Description: Verify TAG version consistency across Dockerfiles without using pip3 # Search for all TAG arguments in Dockerfiles and list their values rg --type dockerfile 'ARG TAG=' | sort | uniq -cLength of output: 85
Script:
#!/bin/bash # Description: Verify TAG version consistency across Dockerfiles by searching common Dockerfile names # Search for all TAG arguments in files named Dockerfile or starting with Dockerfile rg 'ARG TAG=' --glob 'Dockerfile*' | sort | uniq -cLength of output: 1531
skills/skill-easter-eggs/Dockerfile (1)
2-2
: Approve the use of quotes in the FROM instruction.The addition of quotes around the image reference is a good practice. It ensures consistency and prevents potential issues with special characters in the tag.
skills/skill-tunein/Dockerfile (3)
9-10
: Approved: Improved LABEL instructionsThe addition of quotes around
${VERSION}
and${BUILD_DATE}
in the LABEL instructions is a positive change. This practice ensures correct handling of values that might contain spaces or special characters, improving the overall robustness of the Dockerfile.
Line range hint
23-23
: LGTM: ENTRYPOINT remains appropriateThe ENTRYPOINT instruction correctly specifies the command to launch the skill and hasn't been changed. This is appropriate if the launch command for the skill remains the same.
Line range hint
1-23
: Overall: Good improvements to Dockerfile structure and clarityThe changes made to this Dockerfile are positive steps towards improved maintainability and clarity:
- Updating to a specific version (0.1.0) from 'alpha' indicates a move towards stability.
- Adding quotes around variable expansions improves robustness.
- Simplifying the package installation process suggests better integration with standard package repositories.
These changes align well with the PR objectives of updating to a new version and generally follow good Docker practices. The file's overall quality has been improved as a result of these modifications.
skills/skill-volume/Dockerfile (5)
2-2
: Approved: Added quotes to FROM instructionAdding quotes around the image reference in the FROM instruction is a good practice. It prevents potential issues with special characters or spaces in the TAG variable, improving the robustness of the Dockerfile.
9-10
: Approved: Added quotes to LABEL instructionsAdding quotes around the variable references in the LABEL instructions for version and created date is a good practice. This ensures proper string handling and improves the consistency and reliability of the image metadata.
Line range hint
1-26
: Summary: Dockerfile updates improve versioning and formattingThe changes in this Dockerfile align well with the PR objectives of updating to Debian Trixie and Python 12. The modifications include:
- Updating the TAG from "alpha" to a specific version "0.1.0".
- Adding quotes to improve robustness in FROM and LABEL instructions.
- Maintaining the core functionality of the skill container.
These changes improve the overall quality and consistency of the Dockerfile. Please ensure to run the suggested verification scripts to confirm version consistency across the project and the compatibility of the installation process with the new base image and Python version.
Line range hint
16-26
: Verify: Installation process and entry point unchangedThe RUN instruction for installing the skill and the ENTRYPOINT instruction remain unchanged. This suggests that the core functionality of the skill container is maintained, which is good for compatibility. However, it's important to verify that the installation process still works correctly with the updated base image and Python version.
To ensure the installation process is still valid, please run the following script:
#!/bin/bash # Description: Verify the ovos-skill-volume package availability and compatibility # Test: Check if the package is available for the new Python version pip3 install ovos-skill-volume --dry-run # Test: Check if there's a pre-release version available pip3 install ovos-skill-volume --pre --dry-run # Note: These commands will only check package availability without actually installing. # The actual installation and functionality testing should be done in a controlled environment.
1-1
: Approved: Version update from alpha to 0.1.0The change from
alpha
to a specific version number0.1.0
indicates a move towards a more stable release. This aligns well with the PR objective of updating to Debian Trixie and Python 12.To ensure version consistency across the project, please run the following script:
skills/skill-parrot/Dockerfile (3)
1-1
: Excellent move to explicit versioning!Changing from
alpha
to a specific version number (0.1.0
) is a positive step. This change:
- Improves traceability and reproducibility of builds.
- Indicates a move towards a more stable release cycle.
- Aligns with semantic versioning best practices.
9-10
: Approved: Improved robustness in LABEL instructions.The addition of quotes around
${VERSION}
and${BUILD_DATE}
variables in the LABEL instructions is a positive change:
- It ensures that the values are treated as single strings, even if they contain spaces or special characters.
- This practice prevents potential issues with label parsing or usage in various Docker tools.
- It improves consistency with other quoted strings in the Dockerfile.
These changes align with best practices for Dockerfile LABEL instructions and contribute to more reliable metadata handling.
Line range hint
1-24
: Overall improvements in versioning and formatting.The changes in this Dockerfile represent positive steps towards better versioning and more robust formatting:
- The move from 'alpha' to a specific version (0.1.0) indicates progress in the project's lifecycle and improves traceability.
- Consistent use of quotes around variables and strings enhances robustness and prevents potential issues with special characters or spaces.
- These changes align with best practices for Dockerfile construction without altering the core functionality of the skill.
These improvements contribute to better maintainability and reliability of the build process. Great job on these enhancements!
skills/skill-personal/Dockerfile (5)
1-1
: Approved: Specific version tag improves traceabilityThe change from
alpha
to0.1.0
for theTAG
argument is a positive step. It provides better version control and aligns with the PR's objective of updating the project. This specific version number enhances reproducibility and makes it easier to track which version of the base image is being used.
2-2
: Approved: Quoted image reference enhances robustnessAdding quotes around the image reference in the
FROM
instruction is a good practice. While it doesn't change the functionality, it improves readability and prevents potential issues that could arise from special characters in the tag. This change contributes to the overall robustness of the Dockerfile.
9-10
: Approved: Consistent quoting in LABEL instructionsThe addition of quotes around
${VERSION}
and${BUILD_DATE}
in the LABEL instructions is a positive change. This practice:
- Ensures consistency with the quoting style used in the FROM instruction.
- Prevents potential issues with spaces or special characters in the version or build date.
- Improves the overall robustness and maintainability of the Dockerfile.
While these changes don't affect functionality, they contribute to better practices in Dockerfile writing.
Line range hint
1-24
: Summary: Improved versioning and formatting with maintained functionalityOverall, the changes to this Dockerfile are positive:
- The specific version tag (0.1.0) improves traceability and reproducibility.
- Consistent use of quotes in FROM and LABEL instructions enhances robustness.
- The core functionality of the Dockerfile is preserved.
These changes align well with the PR objectives of updating the project. The only point for further consideration is the relevance of the ALPHA conditional installation given the new versioning approach.
Great job on improving the Dockerfile structure and consistency!
Line range hint
15-24
: Consider reviewing the ALPHA conditional installationGiven that the TAG has been changed from 'alpha' to a specific version '0.1.0', it might be worth reconsidering the necessity of the ALPHA conditional installation.
Currently, the Dockerfile allows for installing either the stable or pre-release version of ovos-skill-personal based on the ALPHA argument. With a specific version tag, you might want to ensure that this conditional logic still aligns with your versioning strategy.
Consider the following questions:
- Is the ALPHA condition still relevant with the specific version tag?
- Should the installation always use the stable version corresponding to the 0.1.0 tag?
- If ALPHA is still needed, should its default value be changed to align with the new versioning approach?
To help with this decision, you can run the following command to check if and how the ALPHA argument is used elsewhere in the project:
skills/skill-alerts/Dockerfile (3)
1-1
: Excellent update to a specific version tag!Changing the default TAG from 'alpha' to '0.1.0' is a great improvement. This change:
- Enhances reproducibility of builds.
- Provides better version control.
- Reduces potential issues related to using an 'alpha' version in production.
9-10
: Excellent addition of quotes to LABEL values!Adding quotes around the ${VERSION} and ${BUILD_DATE} variables in the LABEL instructions is a great improvement. This change:
- Ensures proper handling of values that may contain spaces or special characters.
- Improves consistency with best practices for Dockerfile LABEL syntax.
- Reduces the risk of potential issues when label values are used or parsed.
Line range hint
1-24
: Overall, excellent improvements to versioning and formatting!The changes in this Dockerfile are well-considered and improve the overall quality of the build process:
- Specifying a concrete version (0.1.0) instead of 'alpha' enhances reproducibility and stability.
- Adding quotes to variable expansions in LABEL instructions improves robustness.
- The core functionality of the Dockerfile remains unchanged, minimizing the risk of new issues.
These updates align with Docker best practices and should lead to more consistent and reliable builds.
skills/skill-ggwave/Dockerfile (1)
9-10
: LGTM: Proper quoting in LABEL instructions.Adding quotes around the VERSION and BUILD_DATE variables in the LABEL instructions is a good practice. It ensures that values with spaces or special characters are properly handled.
skills/skill-wolfie/Dockerfile (4)
2-2
: Approved: Added quotes to base image referenceAdding quotes around the base image reference is a good practice. It improves consistency and prevents potential issues with special characters in image names or tags.
9-10
: Approved: Added quotes to label valuesAdding quotes around the label values for version and created date is a good practice. It improves consistency and prevents potential issues with special characters in the values.
18-20
: Approved: Simplified and standardized installation commandsThe installation commands have been simplified and standardized, which is a good improvement. This change suggests that
skill-wolfie
is now available in a package repository, which is a more stable and maintainable approach.To ensure the package is available and installable, please run the following script:
#!/bin/bash # Description: Verify the availability and installability of the skill-wolfie package # Test: Check if the package is available pip install skill-wolfie --dry-run # Test: Check if the pre-release version is available pip install skill-wolfie --pre --dry-run
1-1
: Approved: Version update from 'alpha' to '0.1.0'This change indicates a move towards a more stable release version, which aligns with the PR objectives.
To ensure version consistency across the project, please run the following script:
✅ Verification successful
Let's proceed to verify the version consistency across all Dockerfiles by running the following script:
Let's perform a more targeted search for the
ARG TAG=
lines in all Dockerfiles by using the following script:
Verified: Version consistency across all Dockerfiles confirmed
All Dockerfiles have been updated to
ARG TAG=0.1.0
, ensuring consistent versioning throughout the project.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for version consistency across Dockerfiles # Test: Search for TAG arguments in all Dockerfiles rg --type dockerfile 'ARG TAG='Length of output: 221
Script:
#!/bin/bash # Description: Verify the presence of ARG TAG in all Dockerfiles rg --type dockerfile '^ARG TAG='Length of output: 69
Script:
#!/bin/bash # Description: Verify the presence of ARG TAG in all Dockerfiles # Find all files named 'Dockerfile' and search for 'ARG TAG=' within them fd 'Dockerfile' --type f -x rg '^ARG TAG=' {}Length of output: 453
skills/skill-persona/Dockerfile (2)
1-1
: LGTM: Version update to semantic versioningThe change from
alpha
to0.1.0
indicates a shift towards a more stable, semantic versioning scheme. This is a good practice for version management and clarity.
9-10
: LGTM: Improved label value handlingAdding quotes around the variable references in the LABEL instructions is a good practice. It prevents potential issues with special characters or spaces in the VERSION and BUILD_DATE values, improving the robustness of the Dockerfile.
skills/skill-homescreen/Dockerfile (3)
2-2
: Approved: Quotes added to FROM instruction value.Adding quotes around the FROM instruction value is a good practice. It ensures consistency and prevents potential issues with special characters in image names or tags.
9-9
: Approved: Quotes added to VERSION variable in LABEL instruction.Adding quotes around the ${VERSION} variable in the LABEL instruction is a good practice. It ensures that the value is treated as a string, even if it contains spaces or special characters, improving the robustness of the Dockerfile.
10-10
: Approved: Quotes added to BUILD_DATE variable in LABEL instruction.Adding quotes around the ${BUILD_DATE} variable in the LABEL instruction is a good practice. It ensures that the value is treated as a string, even if it contains spaces or special characters, improving the robustness of the Dockerfile. This change is consistent with the modification made to the VERSION label.
skills/skill-fallback-unknown/Dockerfile (3)
2-2
: Approved: Quoting improves consistency.Adding quotes around the FROM instruction argument is good for consistency with other quoted strings in the Dockerfile. While not strictly necessary here, it's a harmless change that could prevent issues if special characters are introduced in the future.
9-9
: Approved: Quoting ${VERSION} improves robustness.Adding quotes around the ${VERSION} variable in the LABEL instruction is a good practice. It ensures correct handling of the value even if it contains spaces or special characters.
10-10
: Approved: Quoting ${BUILD_DATE} improves robustness.Adding quotes around the ${BUILD_DATE} variable in the LABEL instruction is a good practice. It ensures correct handling of the value even if it contains spaces or special characters, consistent with the change made to the VERSION label.
skills/skill-jokes/Dockerfile (5)
1-1
: LGTM: Specific version tag is an improvement.Changing from 'alpha' to a specific version number '0.1.0' is a good practice. It provides better version control and clarity about which version of the base image is being used.
2-2
: LGTM: Quoting the image reference enhances reliability.Adding quotes around the image reference in the FROM instruction is a good practice. It helps prevent potential issues that could arise from special characters or spaces in image names or tags.
9-10
: LGTM: Quoting label values improves reliability.Adding quotes around the VERSION and BUILD_DATE variables in the LABEL instructions is a good practice. It ensures proper parsing of these values, especially if they might contain spaces or special characters.
23-23
: Verify ENTRYPOINT for the new package.The ENTRYPOINT instruction references 'skill-ovos-icanhazdadjokes.openvoiceos'. While this seems to align with the new package name used in the installation, it's important to verify that this is indeed the correct entry point for the new 'ovos-skill-icanhazdadjokes' package.
To verify the correct entry point, you can run the following script:
#!/bin/bash # Description: Check the entry point in the installed package # Test: Look for the entry point in the package metadata pip3 install ovos-skill-icanhazdadjokes pip3 show ovos-skill-icanhazdadjokes | grep -i "entry-point"Please confirm that 'skill-ovos-icanhazdadjokes.openvoiceos' is the correct entry point for the new package.
18-20
:⚠️ Potential issuePackage installation changes need clarification.
The package name has changed from 'ovos-skill-jokes' to 'ovos-skill-icanhazdadjokes', which seems to be a more specific version of the jokes skill. However, there are a few points that need attention:
The installation command is now the same for both ALPHA=true and ALPHA=false conditions. Is this intentional? It removes the flexibility to install different versions based on the ALPHA flag.
The '--pre' flag is still used when ALPHA is true. This might install pre-release versions, which could be unstable. Is this the intended behavior?
The conditional structure (if-else) is now redundant since both branches execute the same command. Consider simplifying this to a single RUN instruction.
To verify the package availability and its versions, you can run the following script:
Could you please clarify the reasoning behind these changes and confirm if they align with the intended behavior?
skills/skill-hello-world/Dockerfile (3)
2-2
: Approved: Quoting the base image reference improves robustness.Adding quotes around the base image reference is a good practice. It ensures consistency and prevents potential issues with special characters in image names or tags.
9-9
: Approved: Quoting the VERSION variable improves robustness.Adding quotes around the ${VERSION} variable in the LABEL instruction is a good practice. It ensures that the version string is correctly handled, even if it contains spaces or special characters.
10-10
: Approved: Quoting the BUILD_DATE variable improves robustness.Adding quotes around the ${BUILD_DATE} variable in the LABEL instruction is a good practice. It ensures that the date string is correctly handled, even if it contains spaces or special characters.
skills/skill-randomness/Dockerfile (2)
1-1
: LGTM: Version update from alpha to specific versionUpdating the TAG from 'alpha' to '0.1.0' is a good practice. It provides better version control and reproducibility for the build process.
9-10
: LGTM: Improved LABEL instructionsAdding quotes around the ${VERSION} and ${BUILD_DATE} variables in the LABEL instructions is a good practice. It ensures that the values are properly handled even if they contain spaces or special characters.
skills/skill-wikipedia/Dockerfile (3)
2-2
: Approved: Quoting the image reference improves robustness.Adding quotes around the image reference is a good practice. It prevents potential issues with special characters and improves consistency with other quoted strings in the Dockerfile.
9-10
: Approved: Quoting label values enhances robustness.Adding quotes around the ${VERSION} and ${BUILD_DATE} variables in the LABEL instructions is a good practice. This ensures proper handling of values, especially if they contain spaces or special characters, improving the overall robustness of the Dockerfile.
Line range hint
1-24
: Summary: Good improvements with some suggestions for consideration.Overall, the changes to this Dockerfile improve its robustness and simplify the installation process. The use of quotes for variables and image references enhances reliability, and the simplified installation command is a step in the right direction.
However, consider the suggestions regarding:
- Using a variable for easier version updates.
- Reconsidering the use of '--pre' flag and the removal of the ALPHA logic for more flexible version control.
These adjustments could further improve the Dockerfile's maintainability and flexibility.
skills/skill-fallback-chatgpt/Dockerfile (4)
1-2
: LGTM: Version update and improved formattingThe TAG argument has been updated to version 0.1.0, and quotes have been added around the image reference in the FROM instruction. These changes improve consistency and follow best practices for Dockerfile syntax.
9-10
: LGTM: Improved LABEL formattingQuotes have been added around the VERSION and BUILD_DATE variables in the LABEL instructions. This change improves the robustness of the Dockerfile by ensuring proper formatting and preventing potential issues with special characters in these values.
28-28
: LGTM: Improved WORKDIR formattingQuotes have been added around the WORKDIR path. This change improves the robustness of the Dockerfile by ensuring proper handling of potential spaces or special characters in the path.
Line range hint
1-28
: Overall improvements with a minor concernThe changes in this Dockerfile primarily focus on improving formatting, consistency, and package management. The use of quotes around variables and paths enhances robustness, while the shift from git-based installation to package-based installation suggests a more stable release process.
These changes are positive and align with best practices for Dockerfile creation. However, there's one point of concern:
- The potential typo in the alpha version package name ('skill-ovos-fallback-chatgptt' with an extra 't') needs verification.
To ensure all changes are correct and intentional, please:
- Confirm the correct package name for the alpha version.
- Verify that all quoted variables (${TAG}, ${VERSION}, ${BUILD_DATE}, ${USER}) are correctly defined and passed to the Docker build process.
You can use the following script to check the defined ARGs:
#!/bin/bash # List all ARG instructions in the Dockerfile echo "ARG instructions in the Dockerfile:" grep '^ARG' skills/skill-fallback-chatgpt/Dockerfileskills/skill-date-time/Dockerfile (2)
9-9
: Approve the addition of quotes in LABEL instruction.Adding quotes around the ${VERSION} variable in the LABEL instruction is a good practice. It ensures that the value is treated as a single string, even if it contains spaces or special characters.
10-10
: Approve the addition of quotes in LABEL instruction.Adding quotes around the ${BUILD_DATE} variable in the LABEL instruction is a good practice. It ensures that the value is treated as a single string, even if it contains spaces or special characters.
skills/skill-weather/Dockerfile (6)
2-2
: Approved: Added quotes to FROM instructionAdding double quotes around the image reference in the FROM instruction is a good practice. It improves consistency and can prevent issues with special characters in the TAG variable.
9-10
: Approved: Added quotes to LABEL instructionsAdding double quotes around the variable references in the LABEL instructions is consistent with the earlier change and follows best practices. It improves consistency and can prevent issues with special characters in the VERSION and BUILD_DATE variables.
Line range hint
29-29
: Approved: Cache removal instruction retainedThe instruction to remove the cache after package installation is correctly retained. This is a good practice as it helps to reduce the final image size.
Line range hint
1-31
: Overall: Positive improvements to versioning and consistencyThe changes in this Dockerfile represent positive steps towards improved versioning, formatting consistency, and simplified installation logic. The move from an alpha tag to a specific version number, the addition of quotes to prevent potential issues with special characters, and the simplification of the package installation process are all commendable improvements.
To ensure these changes don't introduce any unforeseen issues, please:
- Verify the compatibility of the weather skill with the new base image version.
- Confirm that the new installation method doesn't negatively impact your development workflow, especially for testing unreleased changes.
- Test the built image to ensure it functions as expected.
These verifications will help maintain the reliability and efficiency of your build process while benefiting from the improvements introduced in this update.
1-1
: Approved: Version update from alpha to 0.1.0The change from 'alpha' to a specific version number (0.1.0) indicates a move towards a more stable release. This is a positive step in version management.
Please ensure that the weather skill is compatible with the new base image version. Run the following script to check the base image's Python version:
#!/bin/bash # Description: Verify Python version in the base image # Test: Check Python version in the base image docker pull smartgic/ovos-skill-base:0.1.0 docker run --rm smartgic/ovos-skill-base:0.1.0 python3 --version
25-28
: Approved: Simplified package installation logicThe change simplifies the installation process by directly using pip instead of installing from a GitHub repository. This is generally a good practice as it reduces dependencies on external repositories.
However, please verify that this change doesn't negatively impact your development workflow, especially for testing the latest unreleased changes.
Run the following script to check the installed version of ovos-skill-weather:
✅ Verification successful
</run_scripts>
Verified: '--pre' flag is properly used in the installation command
The
Dockerfile
correctly includes the--pre
flag when installingovos-skill-weather
, ensuring that pre-release versions are installed as intended. This change simplifies the installation process without compromising functionality.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify installed version of ovos-skill-weather # Test: Build the image and check the installed version docker build -t weather-skill:test . docker run --rm weather-skill:test pip show ovos-skill-weatherLength of output: 344
Script:
#!/bin/bash # Description: Inspect Dockerfile for ovos-skill-weather installation details # Extract lines 25-28 from the Dockerfile echo "Extracting lines 25-28 from skills/skill-weather/Dockerfile:" sed -n '25,28p' skills/skill-weather/Dockerfile # Check if '--pre' flag is used in the installation command echo "" echo "Checking for '--pre' flag in ovos-skill-weather installation:" sed -n '25,28p' skills/skill-weather/Dockerfile | grep -- '--pre' && echo "'--pre' flag is present." || echo "'--pre' flag is missing."Length of output: 606
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.
Will run the different containers and report any issues found in separate comments
@@ -3,7 +3,7 @@ | |||
# tts.list file is deprecated and will be removed | |||
tts_list=~/.config/mycroft/tts.list | |||
audio_list=~/.config/mycroft/audio.list | |||
audio_list_state=/tmp/audio.state | |||
audio_list_state=~/.local/state/audio.state |
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.
Can we save these into a mycroft or ovos folder?
audio_list_state=~/.local/state/mycroft/audio.state
That way I only have to p[ass on that mycroft folder as volume intro the container instead of the overall users state directory itself.
ovos-workshop | ||
pyalsaaudio |
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.
Thx, this will solve #60
@@ -3,7 +3,7 @@ | |||
# stt.list file is deprecated and will be removed | |||
stt_list=~/.config/mycroft/stt.list | |||
listener_list=~/.config/mycroft/listener.list | |||
listener_list_state=/tmp/listener.state | |||
listener_list_state=/.local/state/listener.state |
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.
See above...
@@ -2,10 +2,10 @@ | |||
|
|||
# Install PHAL admin plugins via pip command when a setup.py exists | |||
phal_admin_list=~/.config/mycroft/phal_admin.list | |||
phal_admin_list_state=/tmp/phal_admin.state | |||
phal_admin_list_state=~/.local/state/phal_admin.state |
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.
Ditto
@@ -2,10 +2,10 @@ | |||
|
|||
# Install PHAL plugins via pip command when a setup.py exists | |||
phal_list=~/.config/mycroft/phal.list | |||
phal_list_state=/tmp/phal.state | |||
phal_list_state=~/local/state/phal.state |
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.
Same same
ovos-phalI run the ovos-phal container with below phal.list file
I believe the oauth plugin requires Pillow and QRcode packages however Pillow can not be installed at runtime of the container. Perhaps it is best to pre-install and ship with Pillow? I saw it was there before but got removed later? Below the last section of the rather long error log.
|
ovos-coreI run the ovos-core image with the below skills.list;
All seems to work OK with only te below one error line about Padatious not being installed;
|
ovos-audioovos-audio also seems to work as expected. I run the container with the below audio.list;
Which only give the below known error bout the g2p mimic plugin;
|
ovos-listenerRun it with below listener.list;
All good, see below full log;
|
ovos-gui-websocketRunning all fine with one ERROR which is not a big deal and easy to fix;
Just a matter of mkdir the /etc/xdg directory within the image. |
ovos-gui-shellAll good;
|
ovos-messagebusAll good;
|
|
should probably be present in PHAL and skills, not needed in sound |
Closing Alpine tentative as TensorFlow doesn't compile with MUSL.
Summary by CodeRabbit
Release Notes
New Features
0.0.8a
to0.1.0
across documentation and Dockerfiles.requirements.txt
for the GUI with essential dependencies.Bug Fixes
Documentation
README
files for better readability.Chores