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.

Sign up for GitHub

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

Jump to bottom

Adding dbus interface for loading locale keyboards #6093

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Adding dbus interface for loading locale keyboards #6093

wants to merge 2 commits into from

Conversation

adamkankovsky
Copy link
Contributor

No description provided.

@github-actions github-actions bot added the f42 Fedora 42 label Jan 14, 2025
@pep8speaks
Copy link

pep8speaks commented Jan 14, 2025 •
edited
Loading

Hello @adamkankovsky! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 57:1: E731 do not assign a lambda expression, use a def
Line 58:1: E731 do not assign a lambda expression, use a def

Comment last updated at 2025-01-17 12:57:20 UTC

jkonecny12
jkonecny12 requested changes Jan 14, 2025
View reviewed changes
pyanaconda/localization.py Outdated Show resolved Hide resolved
@jkonecny12
Copy link
Member

jkonecny12 commented Jan 14, 2025 •
edited
Loading

Also we shouldn't forget about localization of the layouts
https://github.com/rhinstaller/anaconda/blob/main/pyanaconda%2Fui%2Fgui%2Fxkl_wrapper.py#L32-L33

We can resolve on the UI side or provide a separate API for that. Something like get_localized. Not sure what is better approach

jkonecny12
jkonecny12 requested changes Jan 15, 2025
View reviewed changes
Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this improvement! I would like to know what do you think about the notes below before setting ACK.

pyanaconda/modules/localization/localization.py Outdated Show resolved Hide resolved
pyanaconda/localization.py Outdated Show resolved Hide resolved
pyanaconda/ui/gui/xkl_wrapper.py Show resolved Hide resolved
jkonecny12
jkonecny12 approved these changes Jan 15, 2025
View reviewed changes
Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

I just wonder how we are going to resolve the localization? Will that be DBus API or a UI code?

@adamkankovsky
Copy link
Contributor Author

/kickstart-test --testtype smoke

@adamkankovsky
Copy link
Contributor Author

/kickstart-test --testtype smoke

@adamkankovsky
Creating a dbus interface to get local keyboard layouts
e17ac31 
Adding dbus interface for loading locale keyboards

Remove duplicate function and migrate to localization one
@adamkankovsky adamkankovsky deployed to gh-cockpituous January 17, 2025 12:57 — with GitHub Actions Active
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkonecny12 jkonecny12 jkonecny12 approved these changes

@KKoukiou KKoukiou Awaiting requested review from KKoukiou

Assignees
No one assigned
Labels
f42 Fedora 42
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@adamkankovsky @pep8speaks @jkonecny12