Skip to content
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

requirements:latest stable packages #563

Merged
merged 11 commits into from
Oct 15, 2024
Merged

requirements:latest stable packages #563

merged 11 commits into from
Oct 15, 2024

Conversation

JarbasAl
Copy link
Member

@JarbasAl JarbasAl commented Oct 15, 2024

some unittests moved to OpenVoiceOS/OVOS-workshop#235

brings support for ovos-workshop 1.0.0 , it removed support for running UNDER mycroft-core, this was a breaking change in some imports under the mycroft namespace

Summary by CodeRabbit

  • New Features

    • Updated import paths for skill classes to enhance compatibility with the latest implementation.
    • Added a warning log for users regarding the deprecation of the mycroft module, encouraging transition to ovos_core.
  • Bug Fixes

    • Adjusted access to FallbackMode to ensure proper functionality within the fallback service.
  • Chores

    • Updated version specifications for various dependencies to ensure compatibility with the Mycroft ecosystem.

Copy link

coderabbitai bot commented Oct 15, 2024

Walkthrough

The changes in this pull request primarily involve modifying import statements for the MycroftSkill class and related components across multiple files, transitioning from the ovos_workshop.skills.mycroft_skill module to ovos_workshop.skills.ovos. This adjustment maintains backward compatibility while updating the underlying implementation. Additionally, a deprecation warning is added to inform users about the future removal of the mycroft module. Several requirements files are also updated to reflect changes in package dependencies and version constraints.

Changes

File Path Change Summary
mycroft/__init__.py Added a deprecation warning for the mycroft module.
mycroft/skills/__init__.py Updated __all__ to reflect changes in imports.
mycroft/skills/common_iot_skill.py, mycroft/skills/core.py, mycroft/skills/mycroft_skill/__init__.py, mycroft/skills/mycroft_skill/mycroft_skill.py Changed import from ovos_workshop.skills.mycroft_skill to ovos_workshop.skills.ovos, renaming OVOSSkill to MycroftSkill.
ovos_core/intent_services/fallback_service.py Updated import statement for FallbackMode from ovos_workshop.skills.fallback to ovos_workshop.permissions.
requirements/* Various updates to dependencies, including additions, removals, and version constraint updates across multiple requirements files.

Possibly related PRs

  • feat/pipeline_plugins_opm #527: The changes in this PR involve updating import statements in mycroft/skills/intent_services/adapt_service.py, which reflects a similar shift in the underlying implementation of skill classes as seen in the main PR's modification of the MycroftSkill import.
  • improve_typing #528: This PR updates import statements and enhances type annotations in ovos_core/intent_services/__init__.py and related files, which aligns with the main PR's focus on modifying imports and maintaining backward compatibility in the skill class structure.

Suggested labels

packaging, refactor

Suggested reviewers

  • goldyfruit
  • j1nx
  • builderjer

🐇 In the meadow, skills take flight,
With imports changed, all feels right.
A warning here, a version there,
Mycroft hops on, with skills to share!
🌼 Let's embrace the new, with joy and cheer!


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

ovos-skill-pyradios>=0.1.0,<1.0.0
ovos-skill-local-media>=0.2.0,<1.0.0
ovos-skill-youtube-music>=0.1.1,<1.0.0
Copy link
Member Author

Choose a reason for hiding this comment

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

removed youtube-music because youtube is unstable and keeps breaking, let users decide if they are up to that...

Copy link

codecov bot commented Oct 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.93%. Comparing base (23f0bab) to head (d750f87).
Report is 23 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #563      +/-   ##
==========================================
- Coverage   75.33%   72.93%   -2.41%     
==========================================
  Files          15       15              
  Lines        3094     1633    -1461     
==========================================
- Hits         2331     1191    -1140     
+ Misses        763      442     -321     
Flag Coverage Δ
end2end 53.39% <100.00%> (?)
unittests 51.68% <100.00%> (-23.66%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (9)
requirements/lgpl.txt (1)

2-2: Unchanged fann2 dependency specification.

The fann2 dependency specification remains unchanged. If this line was unintentionally included in the commit, consider removing it to keep the changes focused.

If you decide to keep this line in the commit, no further action is needed as the version constraint is correctly specified.

requirements/mycroft.txt (2)

1-1: Consider clarifying the comment for better readability.

The comment provides good context, but a small adjustment could improve its clarity.

Consider revising the comment as follows:

-# all ovos core modules, a full install like mycroft-core used to do
+# All OVOS core modules for a full installation (similar to mycroft-core)

This revision:

  1. Capitalizes "All" for consistency with standard comment style.
  2. Uses "OVOS" in all caps as it's likely an acronym.
  3. Rephrases for better grammatical structure and clarity.
🧰 Tools
🪛 LanguageTool

[grammar] ~1-~1: The word ‘install’ is not a noun.
Context: # all ovos core modules, a full install like mycroft-core used to do ovos_PHAL[...

(A_INSTALL)


[uncategorized] ~1-~1: A punctuation mark might be missing here.
Context: ...ull install like mycroft-core used to do ovos_PHAL[extras]>=0.2.5,<1.0.0 ovos-aud...

(AI_EN_LECTOR_MISSING_PUNCTUATION)


1-6: Summary: Significant version updates across OVOS core modules

This update includes substantial version increases for several OVOS core modules. While these changes likely bring important improvements and new features, they also represent a significant shift in the minimum required versions.

Consider the following recommendations:

  1. Thoroughly test your project with these updated dependencies to ensure compatibility.
  2. Review the changelogs for all updated modules to understand new features, improvements, and any breaking changes.
  3. Update your project's documentation to reflect these new minimum version requirements.
  4. Consider creating or updating a migration guide for users of your project, especially if any breaking changes are introduced by these updates.
  5. Ensure your CI/CD pipeline is updated to use these new minimum versions in its test environments.
🧰 Tools
🪛 LanguageTool

[grammar] ~1-~1: The word ‘install’ is not a noun.
Context: # all ovos core modules, a full install like mycroft-core used to do ovos_PHAL[...

(A_INSTALL)


[uncategorized] ~1-~1: A punctuation mark might be missing here.
Context: ...ull install like mycroft-core used to do ovos_PHAL[extras]>=0.2.5,<1.0.0 ovos-aud...

(AI_EN_LECTOR_MISSING_PUNCTUATION)

requirements/extra-deprecated.txt (1)

9-10: Approved: pyaudio package relocation

The relocation of the pyaudio package doesn't affect its functionality. However, to improve maintainability, consider adding a comment explaining the reason for this relocation.

Consider adding a comment above the pyaudio line to explain its placement, for example:

+# Audio-related dependencies
pyaudio
mycroft/skills/mycroft_skill/__init__.py (1)

15-15: LGTM! Consider adding a deprecation warning.

The change from ovos_workshop.skills.mycroft_skill to ovos_workshop.skills.ovos aligns with the PR objective of updating to the latest stable packages. This modification maintains backward compatibility by aliasing OVOSSkill as MycroftSkill.

To facilitate a smoother transition in the future, consider adding a deprecation warning:

import warnings

warnings.warn("The 'mycroft_skill' module is deprecated. Use 'ovos_workshop.skills.ovos' directly in the future.", DeprecationWarning, stacklevel=2)
🧰 Tools
🪛 Ruff

15-15: ovos_workshop.skills.ovos.OVOSSkill imported but unused

(F401)

mycroft/skills/core.py (1)

Line range hint 16-20: Consider adding a deprecation warning.

While the file comment indicates that this file is deprecated, it would be beneficial to add a more prominent deprecation warning. This will help ensure that developers are aware of the upcoming changes and can plan accordingly.

Here's a suggested implementation:

import warnings

warnings.warn(
    "The 'mycroft.skills.core' module is deprecated and will be removed in a future version. "
    "Please import directly from 'mycroft.skills' instead.",
    DeprecationWarning,
    stacklevel=2
)

This warning will be visible to developers when they import from this module, encouraging them to update their import statements in the future.

🧰 Tools
🪛 Ruff

23-23: ovos_workshop.skills.ovos.OVOSSkill imported but unused

Remove unused import: ovos_workshop.skills.ovos.OVOSSkill

(F401)


24-24: ovos_workshop.skills.fallback.FallbackSkill imported but unused

Remove unused import: ovos_workshop.skills.fallback.FallbackSkill

(F401)


25-25: mycroft.skills.mycroft_skill.resting_screen_handler imported but unused

Remove unused import

(F401)


25-25: mycroft.skills.mycroft_skill.intent_handler imported but unused

Remove unused import

(F401)


25-25: mycroft.skills.mycroft_skill.intent_file_handler imported but unused

Remove unused import

(F401)


25-25: mycroft.skills.mycroft_skill.skill_api_method imported but unused

Remove unused import

(F401)

mycroft/__init__.py (2)

21-21: Approve import change with a minor suggestion.

The import change from mycroft_skill.MycroftSkill to ovos.OVOSSkill as MycroftSkill is a good step towards transitioning to the new OVOS framework while maintaining backward compatibility. This change aligns well with the deprecation strategy.

Consider adding a comment explaining the reason for this aliasing to help future maintainers understand the transition:

# Alias OVOSSkill as MycroftSkill for backward compatibility
from ovos_workshop.skills.ovos import OVOSSkill as MycroftSkill
🧰 Tools
🪛 Ruff

21-21: ovos_workshop.skills.ovos.OVOSSkill imported but unused

(F401)


21-21: Address static analysis false positive.

The static analysis tool (Ruff) reports that ovos_workshop.skills.ovos.OVOSSkill is imported but unused (F401). This is a false positive because the import is used, but it's aliased as MycroftSkill.

To suppress this false positive, you can add a # noqa: F401 comment at the end of the import line:

from ovos_workshop.skills.ovos import OVOSSkill as MycroftSkill  # noqa: F401

This will tell the linter to ignore the unused import warning for this specific line.

🧰 Tools
🪛 Ruff

21-21: ovos_workshop.skills.ovos.OVOSSkill imported but unused

(F401)

ovos_core/intent_services/fallback_service.py (1)

26-26: LGTM! Consider adding a comment for clarity.

The import change from ovos_workshop.skills.fallback to ovos_workshop.permissions for FallbackMode is correct and aligns with the refactoring of the ovos_workshop package structure. This change doesn't affect the usage of FallbackMode in the code and maintains backward compatibility.

Consider adding a brief comment explaining the reason for this import change, which could help future maintainers understand the context:

# FallbackMode moved to permissions module for better organization
from ovos_workshop.permissions import FallbackMode
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 4e609c8 and a937a11.

📒 Files selected for processing (18)
  • mycroft/init.py (1 hunks)
  • mycroft/skills/init.py (1 hunks)
  • mycroft/skills/common_iot_skill.py (1 hunks)
  • mycroft/skills/core.py (1 hunks)
  • mycroft/skills/mycroft_skill/init.py (1 hunks)
  • mycroft/skills/mycroft_skill/mycroft_skill.py (1 hunks)
  • ovos_core/intent_services/fallback_service.py (1 hunks)
  • requirements/extra-deprecated.txt (1 hunks)
  • requirements/lgpl.txt (1 hunks)
  • requirements/mycroft.txt (1 hunks)
  • requirements/plugins.txt (1 hunks)
  • requirements/requirements.txt (1 hunks)
  • requirements/skills-audio.txt (1 hunks)
  • requirements/skills-desktop.txt (1 hunks)
  • requirements/skills-essential.txt (1 hunks)
  • requirements/skills-gui.txt (1 hunks)
  • requirements/skills-internet.txt (1 hunks)
  • requirements/skills-media.txt (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • requirements/skills-desktop.txt
🧰 Additional context used
🪛 Ruff
mycroft/__init__.py

21-21: ovos_workshop.skills.ovos.OVOSSkill imported but unused

(F401)

mycroft/skills/core.py

23-23: ovos_workshop.skills.ovos.OVOSSkill imported but unused

Remove unused import: ovos_workshop.skills.ovos.OVOSSkill

(F401)

mycroft/skills/mycroft_skill/__init__.py

15-15: ovos_workshop.skills.ovos.OVOSSkill imported but unused

(F401)

mycroft/skills/mycroft_skill/mycroft_skill.py

32-32: ovos_workshop.skills.ovos.OVOSSkill imported but unused

Remove unused import: ovos_workshop.skills.ovos.OVOSSkill

(F401)

🪛 LanguageTool
requirements/mycroft.txt

[grammar] ~1-~1: The word ‘install’ is not a noun.
Context: # all ovos core modules, a full install like mycroft-core used to do ovos_PHAL[...

(A_INSTALL)


[uncategorized] ~1-~1: A punctuation mark might be missing here.
Context: ...ull install like mycroft-core used to do ovos_PHAL[extras]>=0.2.5,<1.0.0 ovos-aud...

(AI_EN_LECTOR_MISSING_PUNCTUATION)

requirements/plugins.txt

[locale-violation] ~4-~4: “server” é um estrangeirismo. É preferível dizer “servidor”.
Context: ...on-plugin>=0.1.0, <1.0.0 ovos-translate-server-plugin>=0.0.2, <1.0.0 ovos-utterance-no...

(PT_BARBARISMS_REPLACE_SERVER)

🔇 Additional comments (46)
requirements/skills-gui.txt (1)

1-1: Approved: Version update for ovos-skill-homescreen

The update to ovos-skill-homescreen>=1.0.2,<2.0.0 is approved. This change significantly increases the minimum required version while allowing for a broader range of compatible versions.

Key points to consider:

  1. The jump from 0.x.x to 1.x.x suggests major improvements or changes in the package.
  2. Existing installations using versions between 0.0.3 and 1.0.1 will need to upgrade, which may require code adjustments.
  3. The wider upper limit (<2.0.0) provides more flexibility but may introduce unexpected behavior if not properly tested.

To ensure a smooth transition, please:

  1. Thoroughly test the integration with versions 1.0.2 and the latest version below 2.0.0.
  2. Update the changelog or documentation to reflect any necessary migration steps for users upgrading from older versions.
  3. Verify that all dependent code is compatible with the new version range.

Run the following script to check for any usage of ovos-skill-homescreen in the codebase:

This will help identify areas of the code that might need attention due to the version update.

requirements/lgpl.txt (1)

1-1: LGTM! Verify compatibility with the updated ovos_padatious version.

The minimum required version of ovos_padatious has been increased from 0.1.0 to 0.1.2, which is a good practice to keep dependencies up-to-date. This change likely incorporates recent bug fixes or minor improvements.

To ensure this update doesn't introduce any breaking changes, please run the following script to check for any compatibility issues:

requirements/skills-media.txt (5)

2-2: LGTM: Version update for ovos-skill-somafm

The version requirement update from >=0.0.2,<1.0.0 to >=0.1.1,<1.0.0 for ovos-skill-somafm is appropriate. This minor version bump likely includes new features or improvements while maintaining backward compatibility.


3-3: LGTM: Version update for skill-news

The version requirement update from >=0.0.4,<1.0.0 to >=0.1.2,<1.0.0 for skill-news is appropriate. This minor version bump likely includes new features or improvements while maintaining backward compatibility.


5-5: LGTM: Version update for ovos-skill-local-media

The version requirement update from >=0.2.0,<1.0.0 to >=0.2.1,<1.0.0 for ovos-skill-local-media is appropriate. This patch version bump likely includes bug fixes or minor improvements while maintaining backward compatibility.


Line range hint 1-6: Acknowledged: Removal of ovos-skill-youtube-music

The removal of ovos-skill-youtube-music from the requirements file is intentional and well-justified. As mentioned in the previous review comment, YouTube's API is unstable and prone to breaking changes. Allowing users to decide whether to include this skill is a prudent approach, given its potential instability.


Line range hint 1-6: Summary: Requirements updated to latest stable versions

This update to requirements/skills-media.txt achieves the following:

  1. Updates three skills (ovos-skill-somafm, skill-news, and ovos-skill-local-media) to their latest stable versions.
  2. Removes ovos-skill-youtube-music to improve overall stability.

These changes align well with the PR objectives of updating to the latest stable packages. The removal of the YouTube skill and the version updates should result in a more stable and reliable set of media-related skills for users.

requirements/mycroft.txt (5)

2-2: Verify compatibility with updated ovos_PHAL[extras] version.

The minimum version requirement for ovos_PHAL[extras] has been increased from 0.0.5 to 0.2.5, which suggests significant changes or new features.

Please ensure that your project is compatible with these changes. You may want to review the changelog for ovos_PHAL versions 0.1.x and 0.2.x to understand the impact of this update.


3-3: Verify compatibility with updated ovos-audio[extras] version.

The minimum version requirement for ovos-audio[extras] has been significantly increased from 0.0.0 to 0.2.4, indicating major changes or new features.

Please ensure that your project is compatible with these changes. Review the changelog for ovos-audio versions up to 0.2.4 to understand the impact of this update and any potential breaking changes.


4-4: Verify compatibility with updated ovos-gui[extras] version.

The minimum version requirement for ovos-gui[extras] has been increased from 0.0.4 to 0.2.2, suggesting significant changes or new features.

Please ensure that your project is compatible with these changes. Review the changelog for ovos-gui versions 0.1.x and 0.2.x to understand the impact of this update and any potential breaking changes.


5-5: LGTM: Minor version update for ovos-messagebus.

The minimum version requirement for ovos-messagebus has been increased from 0.0.1 to 0.0.6. This is a relatively minor update compared to the others in this file.

While this change is likely to be backwards compatible, it's still good practice to review the changelog for any notable changes or improvements introduced in versions up to 0.0.6.


6-6: Verify compatibility with updated ovos-dinkum-listener[extras] version.

The minimum version requirement for ovos-dinkum-listener[extras] has been significantly increased from 0.0.1 to 0.2.1, indicating major changes or new features.

Please ensure that your project is compatible with these changes. Review the changelog for ovos-dinkum-listener versions up to 0.2.1 to understand the impact of this update and any potential breaking changes.

requirements/skills-audio.txt (7)

2-2: LGTM: Version update for ovos-skill-boot-finished

The version constraint update for ovos-skill-boot-finished from >=0.1.0,<1.0.0 to >=0.2.1,<1.0.0 is appropriate. It ensures the use of a more recent version while maintaining compatibility with the current major version.


3-3: LGTM: Version update for ovos-skill-audio-recording

The version constraint update for ovos-skill-audio-recording from >=0.1.0,<1.0.0 to >=0.2.2,<1.0.0 is appropriate and consistent with the overall pattern of updates in this file.


4-4: LGTM: Version update for ovos-skill-dictation

The version constraint update for ovos-skill-dictation from >=0.1.0,<1.0.0 to >=0.2.0,<1.0.0 is appropriate. Note that the minimum version increase is slightly smaller compared to the other skills (0.2.0 vs 0.2.1 or 0.2.2), which is acceptable as different skills may have different release cycles.


5-5: LGTM: Significant version update for ovos-skill-parrot

The version constraint update for ovos-skill-parrot from >=0.0.1,<1.0.0 to >=0.1.2,<1.0.0 represents a more significant version jump compared to the other skills. This larger increment suggests substantial improvements or additions to the skill. The change is appropriate and maintains compatibility with the current major version.


6-6: LGTM: Version update for ovos-skill-volume

The version constraint update for ovos-skill-volume from >=0.0.2,<1.0.0 to >=0.1.1,<1.0.0 is appropriate. This change, like the previous one, represents a more significant version increment, suggesting notable improvements to the skill while maintaining compatibility with the current major version.


7-7: LGTM: Minor version update for ovos-skill-naptime

The version constraint update for ovos-skill-naptime from >=0.2.3,<1.0.0 to >=0.3.1,<1.0.0 is appropriate. This change represents an increment in the minor version, suggesting more significant updates or new features compared to the other skills in this file. The update maintains compatibility with the current major version.


2-7: Overall: Consistent and appropriate version updates

The changes in this file demonstrate a systematic update of version constraints for various audio-related skills. All updates increase the minimum required version while maintaining the upper limit of <1.0.0, ensuring compatibility with the current major version. These changes likely incorporate bug fixes, improvements, and potentially new features across all skills.

The updates range from small increments (e.g., ovos-skill-boot-finished) to more significant version jumps (e.g., ovos-skill-parrot), suggesting varying degrees of changes in different skills. This approach allows for the integration of the latest stable versions of each skill while minimizing the risk of breaking changes.

requirements/skills-essential.txt (8)

2-2: LGTM: Version update for ovos-skill-fallback-unknown

The minimum version has been updated from 0.0.4 to 0.1.2, which aligns with the PR objective of updating to the latest stable packages.


3-3: LGTM: Version update for ovos-skill-alerts

The minimum version has been updated from 0.0.1 to 0.1.2, which aligns with the PR objective of updating to the latest stable packages.


4-4: LGTM: Version update for ovos-skill-personal

The minimum version has been updated from 0.0.4 to 0.1.4, which aligns with the PR objective of updating to the latest stable packages.


5-5: LGTM: Version update for ovos-skill-date-time

The minimum version has been updated from 0.2.3 to 0.3.2, which aligns with the PR objective of updating to the latest stable packages.


6-6: LGTM: Version update for ovos-skill-hello-world

The minimum version has been updated from 0.0.4 to 0.1.6, which aligns with the PR objective of updating to the latest stable packages.


7-7: LGTM: Version update for skill-wordnet

The minimum version has been updated from 0.0.1 to 0.0.4, which aligns with the PR objective of updating to the latest stable packages.


9-9: LGTM: Version update for ovos-skill-spelling

The minimum version has been updated from 0.1.0 to 0.2.2, which aligns with the PR objective of updating to the latest stable packages.


2-9: Summary: Version updates for essential skills

All changes in this file involve updating the minimum version requirements for various essential skills. These updates align with the PR objective of moving to the latest stable packages. The changes appear to be minor version bumps, which typically indicate new features or non-breaking changes. This should improve the overall functionality and stability of the system.

However, it's important to note that while these changes are likely beneficial, they may introduce new features or behavior changes. To ensure system stability:

  1. Review the changelogs for each updated skill to understand the changes introduced in the new versions.
  2. Consider running integration tests to verify that the updated skills work correctly with the rest of the system.
  3. Monitor the system after deployment for any unexpected behavior or regressions.

To assist with verifying these changes, you can run the following script to check the latest versions of these packages:

This script will help ensure that the versions specified in the requirements file are indeed the latest stable versions available.

requirements/skills-internet.txt (9)

2-2: LGTM: Version update for ovos-skill-weather

The version update from >=0.0.1,<1.0.0 to >=0.1.1,<1.0.0 is appropriate. It allows for new features or non-breaking changes while maintaining backward compatibility and preventing potential breaking changes with the <1.0.0 upper limit.


3-3: LGTM: Version update for skill-ddg

The version update from >=0.0.2,<1.0.0 to >=0.1.2,<1.0.0 is appropriate. It allows for new features or non-breaking changes while maintaining backward compatibility and preventing potential breaking changes with the <1.0.0 upper limit.


4-4: LGTM: Version update for skill-wolfie

The version update from >=0.2.0,<1.0.0 to >=0.2.3,<1.0.0 is appropriate. It allows for bug fixes or very minor updates while maintaining backward compatibility and preventing potential breaking changes with the <1.0.0 upper limit.


5-5: LGTM: Version update for ovos-skill-wikipedia

The version update from >=0.5.0,<1.0.0 to >=0.5.2,<1.0.0 is appropriate. It allows for bug fixes or very minor updates while maintaining backward compatibility and preventing potential breaking changes with the <1.0.0 upper limit.


6-6: LGTM: Version update for skill-ovos-fallback-chatgpt

The version update from >=0.0.2,<1.0.0 to >=0.1.2,<1.0.0 is appropriate. It allows for new features or non-breaking changes while maintaining backward compatibility and preventing potential breaking changes with the <1.0.0 upper limit.


7-7: LGTM: Version update for ovos-skill-wikihow

The version update from >=0.2.0,<1.0.0 to >=0.2.3,<1.0.0 is appropriate. It allows for bug fixes or very minor updates while maintaining backward compatibility and preventing potential breaking changes with the <1.0.0 upper limit.


8-8: LGTM: Version update for ovos-skill-speedtest

The version update from >=0.1.0,<1.0.0 to >=0.2.1,<1.0.0 is appropriate. It allows for new features or non-breaking changes while maintaining backward compatibility and preventing potential breaking changes with the <1.0.0 upper limit.


9-9: LGTM: Version update for ovos-skill-ip

The version update from >=0.1.0,<1.0.0 to >=0.2.2,<1.0.0 is appropriate. It allows for new features or non-breaking changes while maintaining backward compatibility and preventing potential breaking changes with the <1.0.0 upper limit.


2-9: Overall assessment: Consistent and appropriate version updates

The changes in this file demonstrate a consistent approach to updating skill versions. All skills have been updated to newer versions, with a mix of minor version bumps (allowing for new features or non-breaking changes) and patch version bumps (allowing for bug fixes or very minor updates). The maintenance of the <1.0.0 upper limit across all skills is a good practice to prevent potential breaking changes.

These updates should improve the functionality and stability of the skills while maintaining backward compatibility. The consistency in versioning approach across all skills is commendable and makes the system more predictable for users and developers.

requirements/extra-deprecated.txt (1)

11-16: Approved with suggestions: Addition of OVOS-related dependencies

The addition of OVOS-related dependencies for Mycroft namespace compatibility is noted. However, there are a few points to consider:

  1. Given that this is a deprecated requirements file, it's unusual to add new dependencies. Could you clarify the reasoning behind this?

  2. The comment could be more informative. Consider expanding it to explain why these specific packages are needed for Mycroft compatibility.

To ensure these are the correct and most up-to-date versions of these packages, let's verify:

Consider expanding the comment to provide more context:

-# for compat with mycroft namespace
+# The following packages provide compatibility with the Mycroft namespace
+# and are required for seamless integration between OVOS and Mycroft components
✅ Verification successful

Verified: OVOS-related dependencies are up-to-date and appropriately constrained

The added OVOS-related dependencies in the requirements/extra-deprecated.txt file are verified to be up-to-date and appropriately constrained:

  1. All packages have version constraints that include the latest available versions on PyPI.
  2. The constraints allow for updates within major versions (< 1.0.0), which is a good practice for maintaining compatibility while allowing bug fixes and minor improvements.

Regarding the previous suggestions:

  1. The presence of these dependencies in a deprecated file is justified as they provide compatibility with the Mycroft namespace.
  2. The comment could still be expanded for better clarity.

Consider updating the comment as follows:

-# for compat with mycroft namespace
+# The following packages provide compatibility with the Mycroft namespace
+# and are required for seamless integration between OVOS and Mycroft components

This change will provide more context about the purpose of these dependencies in the deprecated requirements file.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the latest versions of the added OVOS packages
# Expected result: The versions in the requirements file should match or be close to the latest versions

echo "Checking latest versions of OVOS packages:"
for package in ovos-listener ovos-tts-plugin-mimic ovos-ww-plugin-precise ovos-ww-plugin-pocketsphinx
do
    latest_version=$(pip index versions $package | head -n 1 | awk '{print $2}' | tr -d '()')
    echo "$package: $latest_version"
done

echo -e "\nVersions in requirements file:"
grep -E "ovos-(listener|tts-plugin-mimic|ww-plugin-precise|ww-plugin-pocketsphinx)" requirements/extra-deprecated.txt

Length of output: 1794


Script:

#!/bin/bash
# Description: Fetch the latest versions of OVOS packages using PyPI's JSON API
# Expected result: List of packages with their latest versions and the versions specified in the requirements file

echo "Fetching latest versions of OVOS packages from PyPI:"

packages=("ovos-listener" "ovos-tts-plugin-mimic" "ovos-ww-plugin-precise" "ovos-ww-plugin-pocketsphinx")

for package in "${packages[@]}"
do
    response=$(curl -s https://pypi.org/pypi/${package}/json)
    if [[ $? -ne 0 || -z "$response" ]]; then
        echo "${package}: Failed to fetch data from PyPI."
        continue
    fi
    latest_version=$(echo "$response" | jq -r '.info.version' 2>/dev/null)
    if [[ $? -ne 0 || -z "$latest_version" ]]; then
        echo "${package}: Unable to parse version from PyPI response."
        continue
    fi
    echo "${package}: Latest version: ${latest_version}"
done

echo -e "\nVersions in requirements file:"
grep -E "ovos-(listener|tts-plugin-mimic|ww-plugin-precise|ww-plugin-pocketsphinx)" requirements/extra-deprecated.txt || echo "No OVOS packages found in requirements/extra-deprecated.txt."

Length of output: 151723

requirements/requirements.txt (4)

7-7: LGTM: Minor version bump for ovos-adapt-parser

The version constraint update for ovos-adapt-parser from >=0.1.1 to >=0.1.2 is appropriate. This minor version bump likely includes new features or non-breaking changes, adhering to semantic versioning principles.


17-17: Verify intention: Major version bump for ovos-workshop

The version constraint update for ovos-workshop from >=0.0.16,<1.0.0 to >=1.0.0,<2.0.0 represents a significant change. This major version bump likely includes breaking changes or substantial new features, and indicates that the package has reached a stable, production-ready state.

However, there's a past review comment mentioning a TODO about reverting the minimum workshop version to 0.0.16. Please confirm if this major version bump is intentional and aligns with the project's current goals.

To help verify the changes between versions, you can run the following script:

#!/bin/bash
# Description: Check for changelog entries for ovos-workshop version 1.0.0

# Test: Search for changelog entries
rg --type md -i "changelog|release notes" | rg -i "ovos.?workshop.*1\.0\.0"

Additionally, you may want to search for any usage of ovos-workshop in the codebase to ensure compatibility with version 1.0.0:

#!/bin/bash
# Description: Search for usage of ovos-workshop in the codebase

# Test: Search for import statements or other usage
rg --type python "from ovos.?workshop import|import ovos.?workshop"

8-8: LGTM: Version bump for ovos_ocp_pipeline_plugin

The version constraint update for ovos_ocp_pipeline_plugin from >=0.1.0 to >=0.1.2 is appropriate. This minor version bump likely includes new features or non-breaking changes.

As the update skips version 0.1.1, it might be worth checking the changelog for any significant updates or features introduced in both 0.1.1 and 0.1.2. Here's a script to help verify:


9-9: LGTM: Version bump for ovos-common-query-pipeline-plugin

The version constraint update for ovos-common-query-pipeline-plugin from >=0.1.0 to >=0.1.2 is appropriate. This minor version bump likely includes new features or non-breaking changes.

As with the previous package, this update skips version 0.1.1. It would be beneficial to check the changelog for any significant updates or features introduced in both 0.1.1 and 0.1.2. Here's a script to help verify:

mycroft/skills/core.py (1)

23-23: LGTM! Import change maintains backward compatibility.

The change from from ovos_workshop.skills.mycroft_skill import MycroftSkill to from ovos_workshop.skills.ovos import OVOSSkill as MycroftSkill is a good approach. It updates the underlying implementation while maintaining backward compatibility for existing skills. This allows for a smoother transition to the new OVOSSkill class.

🧰 Tools
🪛 Ruff

23-23: ovos_workshop.skills.ovos.OVOSSkill imported but unused

Remove unused import: ovos_workshop.skills.ovos.OVOSSkill

(F401)

mycroft/skills/mycroft_skill/mycroft_skill.py (1)

32-32: Approve change with suggestions for improvement

The change to import OVOSSkill as MycroftSkill is a good approach for maintaining backward compatibility while transitioning to a new base class. However, I have a few suggestions to improve this change:

  1. Consider adding a deprecation warning to inform users about the future removal of the MycroftSkill alias. This will help developers prepare for future updates.

  2. The static analysis tool suggests that this import is unused. Let's verify if it's necessary.

  3. Update any relevant documentation to reflect this change in the base class for Mycroft skills.

Here's a suggested implementation with a deprecation warning:

import warnings
from ovos_workshop.skills.ovos import OVOSSkill

warnings.warn("MycroftSkill is deprecated and will be removed in a future version. Use OVOSSkill instead.", DeprecationWarning, stacklevel=2)
MycroftSkill = OVOSSkill

Let's verify the usage of MycroftSkill in the codebase:

This script will help us determine if the import is necessary and how widely MycroftSkill is used in the codebase.

🧰 Tools
🪛 Ruff

32-32: ovos_workshop.skills.ovos.OVOSSkill imported but unused

Remove unused import: ovos_workshop.skills.ovos.OVOSSkill

(F401)

mycroft/__init__.py (1)

Line range hint 33-34: Approve deprecation warning.

The added deprecation warning is clear, informative, and provides users with the necessary guidance for future changes. It effectively communicates:

  1. The deprecation of the mycroft module.
  2. The recommendation to start importing from ovos_core and companion packages.
  3. The specific version (0.2.0) when the mycroft module will be removed.

This information is crucial for users to plan their transition to the new framework.

🧰 Tools
🪛 Ruff

19-19: ovos_workshop.intents.IntentBuilder imported but unused

(F401)


19-19: ovos_workshop.intents.Intent imported but unused

(F401)


20-20: ovos_workshop.decorators.intent_handler imported but unused

(F401)


20-20: ovos_workshop.decorators.intent_file_handler imported but unused

(F401)


20-20: ovos_workshop.decorators.adds_context imported but unused

(F401)


20-20: ovos_workshop.decorators.removes_context imported but unused

(F401)


21-21: ovos_workshop.skills.ovos.OVOSSkill imported but unused

(F401)


22-22: ovos_workshop.skills.fallback.FallbackSkill imported but unused

(F401)

mycroft/skills/__init__.py (1)

25-25: Verify compatibility and consider adding a deprecation warning

The change from MycroftSkill to OVOSSkill (aliased as MycroftSkill) maintains backward compatibility at the API level. However, there are a few points to consider:

  1. Ensure that OVOSSkill is a perfect drop-in replacement for MycroftSkill to avoid introducing subtle behavioral changes.
  2. Consider adding a deprecation warning to inform users about this change and potential future removal of the MycroftSkill alias. This will help with a smoother transition in the future.

Consider adding a deprecation warning like this:

import warnings

warnings.warn(
    "MycroftSkill is deprecated and will be removed in a future version. "
    "Use OVOSSkill from ovos_workshop.skills.ovos instead.",
    DeprecationWarning,
    stacklevel=2
)

To ensure this change doesn't introduce unexpected issues, let's verify the usage and compatibility:

mycroft/skills/common_iot_skill.py (1)

27-27: LGTM! Consider updating documentation.

The change from MycroftSkill to OVOSSkill looks good and maintains backward compatibility through aliasing. This shift aligns with the transition to more generalized OVOS implementations.

To ensure full compatibility, please verify that OVOSSkill provides all the necessary methods and attributes that CommonIoTSkill expects from MycroftSkill. You can run the following script to compare the two classes:

Consider updating the class docstring for CommonIoTSkill to reflect that it now inherits from OVOSSkill instead of MycroftSkill. This will help maintain accurate documentation for developers working with this class in the future.

mycroft/skills/mycroft_skill/__init__.py Show resolved Hide resolved
@JarbasAl JarbasAl merged commit 6d2f8c7 into dev Oct 15, 2024
11 of 12 checks passed
@JarbasAl JarbasAl deleted the fixlatest_packages branch October 15, 2024 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant