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

SDDM theme with Autologin #381

Closed
wants to merge 2 commits into from
Closed

SDDM theme with Autologin #381

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 14, 2024

Pull Request

SDDM Astronaut theme with Autologin

This script installs sddm-astronaut-theme from last stream and provides an option to enable autologin with sddm. Users can choose which desktop session they want to autologin on startup. After selecting desktop session, it will do a sddm restart and autologin to that desktop.

Testing

If you want to test it first, do the following.

git clone --branch sddm https://github.com/mashrukk/linutil
cd linutil && cargo run

Type of Change

  • New feature
  • Bug fix
  • Documentation Update
  • Refactoring
  • Hotfix
  • Security patch
  • UI/UX improvement

Checklist

  • My code adheres to the coding and style guidelines of the project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • My changes generate no errors/warnings/merge conflicts.

@adamperkowski
Copy link
Collaborator

And please don't open any more PRs for the same thing

@nnyyxxxx
Copy link
Contributor

@mashrukk Don't make any more changes to this. It's perfect how it is now.

Copy link
Contributor

@nnyyxxxx nnyyxxxx 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 at the time of reviewing; might be different if anything changes. (needs testing aswell)

@nnyyxxxx
Copy link
Contributor

@mashrukk These are the last 2 changes we request of you, thank you for your contribution otherwise.

@adamperkowski
Copy link
Collaborator

adamperkowski commented Sep 14, 2024

@mashrukk If you'd give me write permissions on your branch, I could squash those commits into one. In your settings

Copy link
Collaborator

@adamperkowski adamperkowski left a comment

Choose a reason for hiding this comment

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

As @nnyyxxxx said: this needs testing, the commits need squashing. (not quoting)

Code looks good for now, although this might change after the tests. I could test this out later when I'm able to, then I'll approve the changes.

@ghost
Copy link
Author

ghost commented Sep 14, 2024

@adamperkowski check invite

@adamperkowski
Copy link
Collaborator

@mashrukk got it

Copy link
Collaborator

@adamperkowski adamperkowski left a comment

Choose a reason for hiding this comment

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

After c5c06fd & 581c243 everything works as expected.

Tested on: Debian 12.6, Ubuntu 24.04 LTS & Arch 2024.09.01

@ghost
Copy link
Author

ghost commented Sep 15, 2024

@adamperkowski To check dependencies required visit sddm-astronaut-theme

@guruswarupa
Copy link
Contributor

guruswarupa commented Sep 15, 2024

Duplicate of #228 partially

@ghost
Copy link
Author

ghost commented Sep 15, 2024

Duplicate of #228 partially

@guruswarupa I appreciate your work. Instead of making a separate autologin script. It's better to have SDDM with auto login functionality.

@guruswarupa
Copy link
Contributor

guruswarupa commented Sep 15, 2024

Duplicate of #228 partially

@guruswarupa I appreciate your work. But Instead of making a separate autologin script. It's better to have SDDM with auto login functionality.

my script auto-login is for multiple display managers. this pr should just work on setting up the theme and not adding features such as autologin which is part of util features, wherein user can choose to keep autologin on or not. what if he doesn't need autologin but wants to have the theme setup? hope you get my point.

@guruswarupa
Copy link
Contributor

Duplicate of #228 partially

@guruswarupa I appreciate your work. But Instead of making a separate autologin script. It's better to have SDDM with auto login functionality.

my script auto-login is for multiple display managers. this pr should just work on setting up the theme and not adding features such as autologin which is part of util features, wherein user can choose to keep autologin on or not. what if he doesn't need autologin but wants to have the theme setup? hope you get my point.

whats your take on it? @adamperkowski @nnyyxxxx

@ghost
Copy link
Author

ghost commented Sep 15, 2024

Duplicate of #228 partially

@guruswarupa I appreciate your work. But Instead of making a separate autologin script. It's better to have SDDM with auto login functionality.

my script auto-login is for multiple display managers. this pr should just work on setting up the theme and not adding features such as autologin which is part of util features, wherein user can choose to keep autologin on or not. what if he doesn't need autologin but wants to have the theme setup? hope you get my point.

@guruswarupa Check the script. It doesn't just set up a theme. It installs SDDM with the theme and asks the user if they want to auto-login, which is optional.

@guruswarupa
Copy link
Contributor

guruswarupa commented Sep 15, 2024

@mashrukk yeah fine, also restarting sddm without asking user is unexpected behavior for script to do.

@guruswarupa
Copy link
Contributor

@guruswarupa That's a good point. But, If it's executed from TTY and we don't restart. The user will have to type the command enable --now Right? In that case we can just enable it. I need your inputs @adamperkowski @nnyyxxxx

if its added to the sddm.conf, automatically when user restarts the theme should be loaded. in the sddm install add sudo systemctl enable sddm.service

@ghost ghost closed this Sep 16, 2024
@ghost ghost reopened this Sep 16, 2024
@ghost ghost closed this Sep 16, 2024
@ghost ghost mentioned this pull request Sep 16, 2024
This pull request was closed.
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