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

Fix/qml resources #135

Merged
merged 3 commits into from Jun 6, 2022
Merged

Fix/qml resources #135

merged 3 commits into from Jun 6, 2022

Conversation

NeonJarbas
Copy link

fix/non_lang_resources - follow up to #130

the resource file refactor was not handling non-lang resources properly, it would miss top level folders that are not localized

QML files were completely ignored in the mk2 refactor, this commit adds "new style" support for handling of qml files

if x.is_file() and res_name == x.name:
return x

for directory in locate_base_directories(root_dir, res_dirname):
Copy link
Member

Choose a reason for hiding this comment

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

@NeonDaniel this should fix the bug you reported in chat

@JarbasAl JarbasAl requested review from NeonDaniel and AIIX June 5, 2022 14:29
@JarbasAl JarbasAl added bug Something isn't working refactor code refactor without functional changes bug-ovos bug introdues in ovos-core, not present in mycroft-core labels Jun 5, 2022
@AIIX
Copy link
Member

AIIX commented Jun 6, 2022

QML files are been resolved from a deeper level than the UI folder, it should only resolve the UI folder not deeper levels, for example its resolving for timer skill the "ui/+mediacenter/Timer.qml" where it should only be resolving "ui/Timer.qml"

the resource file refactor was not handling non-lang resources properly, it would miss top level folders that are not localized

QML files were completely ignored in the mk2 refactor, this commit adds "new style" support for handling of qml files
@codecov
Copy link

codecov bot commented Jun 6, 2022

Codecov Report

Merging #135 (8c5404a) into dev (6ceb058) will increase coverage by 2.34%.
The diff coverage is 41.47%.

@@            Coverage Diff             @@
##              dev     #135      +/-   ##
==========================================
+ Coverage   50.35%   52.69%   +2.34%     
==========================================
  Files         119      150      +31     
  Lines       10077     9652     -425     
==========================================
+ Hits         5074     5086      +12     
+ Misses       5003     4566     -437     
Impacted Files Coverage Δ
mycroft/audio/__main__.py 0.00% <0.00%> (ø)
mycroft/client/enclosure/__main__.py 0.00% <0.00%> (ø)
mycroft/client/enclosure/mark1/arduino.py 0.00% <0.00%> (ø)
mycroft/client/enclosure/mark1/eyes.py 0.00% <0.00%> (ø)
mycroft/client/enclosure/mark1/mouth.py 0.00% <0.00%> (ø)
mycroft/client/speech/__main__.py 0.00% <0.00%> (ø)
mycroft/client/speech/hotword_factory.py 0.00% <0.00%> (-88.89%) ⬇️
mycroft/client/speech/service.py 0.00% <0.00%> (ø)
mycroft/client/speech/silence.py 0.00% <0.00%> (-42.86%) ⬇️
mycroft/client/text/__init__.py 0.00% <0.00%> (ø)
... and 116 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 18cd123...8c5404a. Read the comment docs.

qml files should not walk the directory, only check top level
AIIX
AIIX approved these changes Jun 6, 2022
@AIIX AIIX merged commit 9c6f8c9 into OpenVoiceOS:dev Jun 6, 2022
4 of 5 checks passed
@NeonJarbas NeonJarbas deleted the fix/qml_resources branch June 8, 2022 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working bug-ovos bug introdues in ovos-core, not present in mycroft-core refactor code refactor without functional changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants