Skip to content

LocaleManager: set system locale and keyboard via dbus#205

Merged
danirabbit merged 14 commits into
mainfrom
danirabbit/localemanager-locale1setsystem
Jun 18, 2024
Merged

LocaleManager: set system locale and keyboard via dbus#205
danirabbit merged 14 commits into
mainfrom
danirabbit/localemanager-locale1setsystem

Conversation

@danirabbit

@danirabbit danirabbit commented Jun 14, 2024

Copy link
Copy Markdown
Member

As recommended in #147 (comment)

@danirabbit danirabbit requested a review from a team June 14, 2024 17:31
@danirabbit danirabbit marked this pull request as ready for review June 14, 2024 17:31
Comment thread src/Widgets/LocaleSetting.vala Outdated
Comment thread src/LocaleManager.vala Outdated
@ryonakano

Copy link
Copy Markdown
Member

Also, maybe we can update the data/locale.policy.in file?

  • L18 can be removed since we no longer use localectl with pkexec command
  • org.freedesktop.locale1.set-locale in L19 can be removed since we no longer check for permission when set_system_button is clicked

https://github.com/elementary/switchboard-plug-locale/blob/425a6642d3361dd82a6625e3ef45567e07a53fa0/data/locale.policy.in#L18-L19

@danirabbit danirabbit requested a review from ryonakano June 16, 2024 00:20
@danirabbit

Copy link
Copy Markdown
Member Author

@ryonakano thanks for the thorough review! Addressed those feedback points :)

Comment thread src/LocaleManager.vala Outdated
@danirabbit danirabbit requested a review from ryonakano June 17, 2024 16:57

@ryonakano ryonakano left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just one small nitpicking, otherwise looks good and works as expected.

Comment thread src/LocaleManager.vala Outdated
@danirabbit danirabbit requested a review from ryonakano June 18, 2024 16:59
@danirabbit

Copy link
Copy Markdown
Member Author

@ryonakano thanks for catching that!

@ryonakano ryonakano left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code looks good and works as expected. Thank you for your work!

@danirabbit

Copy link
Copy Markdown
Member Author

Thanks for your thorough review!

@danirabbit danirabbit merged commit 2b8aa1e into main Jun 18, 2024
@danirabbit danirabbit deleted the danirabbit/localemanager-locale1setsystem branch June 18, 2024 22:56
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.

3 participants