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

Non-Ubuntu Distro Accessibility and Formatting #548

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Non-Ubuntu Distro Accessibility and Formatting #548

wants to merge 4 commits into from

Conversation

thebitstick
Copy link

  • Fixes for Surface Go - Touchscreen #251 not working on distro without update-rc.d (non-Ubuntu)
  • Newline formatting is not default on Fedora/RHEL, so adding -e beautifies output
  • Checks for Ubuntu before even asking user to install Ubuntu packages

- Fixes for #251 not working on distro without update-rc.d (non-Ubuntu)
- Newline formatting is not default on Fedora/RHEL, so adding `-e` beautifies output
- Checks for Ubuntu before even asking user to install Ubuntu packages
@StollD
Copy link

StollD commented Aug 5, 2019

The .deb packages work on other apt-based distributions though, and not
just on ubuntu. By checking for ubuntu, you wouldn't detect Mint,
Debian or Elementary OS I think (definitly not Debian, idk if the
others change the ID).

I actually made a pull request that does similar (if not the same, lol)
changes (#538), which just checks for the package manager (i.e. if
apt / dpkg is installed .deb packages are used, and if pacman is
installed the arch packages will work).

Feel free to rip those changes out of my PR in order to simplify
merging this if you want.

@thebitstick
Copy link
Author

Oh! I'm sorry about that! I'll include the changes from b4ddc70 in this PR!
( off note but thank you for the fedora packaging! )

@StollD
Copy link

StollD commented Aug 7, 2019

Nice!

Oh! I'm sorry about that!

No worries! But actually, one thing I still want to suggest:

In #538 I reorgnized the script a bit so it installs libwacom right
before installing the kernel. That allowed me to use only one if / else
check for the package manager, instead of two (that need to be kept in
sync). Maybe thats something you should add too, to have less
duplicated code?

@StollD
Copy link

StollD commented Aug 17, 2019

Another note, because I just read that in the planned changelog:
Fedora 31 won't contain the yum wrapper anymore, so checking for that
will fail. The script should check for dnf instead (even if someone
is crazy enough to use these packages on CentOS / RHEL, they are
switching to dnf too - RHEL already did iirc.)

Sorry for nagging xD

thebitstick added 2 commits October 14, 2019 23:37
For libwacom and kernel installation
@thebitstick
Copy link
Author

Alright it should be ready to merge!

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.

2 participants