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

Added Flatpak guide and some further changes to guides.md #171

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

Conversation

abhigit23
Copy link

@abhigit23 abhigit23 commented Apr 13, 2022

  1. Updated fedora workstation/spins guide : As mpv is not available in the main official fedora repos, we've to add RPM-Fusion repos.
  2. Added Flatpak guide.
  3. Added new version(latest) screenshot in "Usage Instructions for Anime4K section"
  4. Added a note and a tip for fedora and flatpak guide.

Updated fedora workstation/spins guide : As mpv is not available in main official fedora repos, we've to add RPM-Fusion repos.
Added Flatpak guide.
Added new version(latest) screenshot in Usage Instructions for Anime4K section
Add a note and a tip.
@WasteOfO2
Copy link
Contributor

I think RPM-Fusion Free comes enabled by default for Workstation, could be wrong

It is probably better the keep "free" in, so that it is clear which repo to install.

The note abt "more info here" is fine

Seems a bit redundant to create another mpv directory inside of .../config, keeping it as is fine.

Line 99: Keep it as "newer" instead of "older/newer", as this applies for that version and beyond, not the older ones

No other problems that I see otherwise.

A good first PR :)

@abhigit23
Copy link
Author

abhigit23 commented Apr 13, 2022

First of all, thanks for reviewing my PR and giving your valuable time.

I think RPM-Fusion Free comes enabled by default for Workstation, could be wrong

As for RPM-Fusion repo, it's not enabled by default. I know because I'm using fedora workstation, writing this comment from it xD

It is probably better the keep "free" in, so that it is clear which repo to install.

I didn't find any tutorial of adding just "free" repo only, so to give more info to the users I stated what the command actually does. It adds both "free" and "non-free" repos to the fedora.

The note abt "more info here" is fine

Thanks, but if you find it redundant too, I can change it.

Seems a bit redundant to create another mpv directory inside of .../config, keeping it as is fine.

If you're talking about the official repo mpv, mpv folder doesn't exist in ~/.config directory when you install it for the first time. Once you install mpv then play a video it will auto create the mpv folder. Btw it's also the same for flatpak mpv. Still you find it not so clear I'll change it to what you'd say :-)

Line 99: Keep it as "newer" instead of "older/newer", as this applies for that version and beyond, not the older ones

Thanks for pointing that, I'll change it to "newer" or maybe wait until admin give us some feedback. xD

@bloc97
Copy link
Owner

bloc97 commented Apr 13, 2022

Looks great. As I am not familiar with Fedora, we can merge when you think it is ready and tweak it before the next release version if necessary.
Edit: And yes, "newer" by itself is fine as older versions of Anime4K releases should always be linked to a corresponding older version of the instructions in the commit history.

@WasteOfO2
Copy link
Contributor

First of all, thanks for reviewing my PR and giving your valuable time.

I think RPM-Fusion Free comes enabled by default for Workstation, could be wrong

As for RPM-Fusion repo, it's not enabled by default. I know because I'm using fedora workstation, writing this comment from it xD

It is probably better the keep "free" in, so that it is clear which repo to install.

I didn't find any tutorial of adding just "free" repo only, so to give more info to the users I stated what the command actually does. It adds both "free" and "non-free" repos to the fedora.

The note abt "more info here" is fine

Thanks, but if you find it redundant too, I can change it.

Seems a bit redundant to create another mpv directory inside of .../config, keeping it as is fine.

If you're talking about the official repo mpv, mpv folder doesn't exist in ~/.config directory when you install it for the first time. Once you install mpv then play a video it will auto create the mpv folder. Btw it's also the same for flatpak mpv. Still you find it not so clear I'll change it to what you'd say :-)

Line 99: Keep it as "newer" instead of "older/newer", as this applies for that version and beyond, not the older ones

Thanks for pointing that, I'll change it to "newer" or maybe wait until admin give us some feedback. xD

Np mate :)

I think rest of the stuff seems fine, should we add Snap Support while we are at it? Idt a lot of ppl use Snaps, but still it shouldn't be a reason to exclude em

@abhigit23
Copy link
Author

I think rest of the stuff seems fine, should we add Snap Support while we are at it? Idt a lot of ppl use Snaps, but still it shouldn't be a reason to exclude em

I don't know much about snaps because I never used them.
Although I tried to find mpv player on snapcraft website and I found several fork apps of mpv player including the one which I found official(not sure though) was unmaintained, last updated on year 2017.
Also, Ubuntu and Debian mostly ships with older version(v0.33.1 and v0.32 respectively) of mpv, so in the end user have to either build mpv from the sources or use flatpak for the latest version. One more option is to add user PPA(Ubuntu only).

I'll add a note about this issue, as we stated to use mpv build not prior to the June 2021 in the guide, both versions are old and might not work with current shaders.
One last thing, in my opinion mpv from flatpak would be easy to install and config without much hassle. It should be the recommended way to install mpv on Debian/Ubuntu derivatives.
I'll update the guide soon.

Thanks.

Added a note for Debian/Ubuntu distro.
Added flatpak setup guide link.

### Arch and Derivatives
sudo pacman -S mpv

### OpenSUSE Tumbleweed

Choose a reason for hiding this comment

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

Do we really need to teach a linux user to use the Package Manager to install a software?

Choose a reason for hiding this comment

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

Do we really need to teach a linux user to use the Package Manager to install a software?

why not, there's already instructions for others package managers for other distro ? what about newbies that rely more on GUI for package manager ? This kind of elitism has no place in a PR comment imo.

Choose a reason for hiding this comment

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

elitism? It's boring game that taging sb with the strange labels.

@hooke007
Copy link

I think we could simplify some unnecessary ins.
We don't need to list more package-managers. If you have installed Arch, how could it be possible that you don't know how to use pacman?
Only if the common installation is "buggy" (i.e. debian), we should point it out.

@WasteOfO2
Copy link
Contributor

WasteOfO2 commented Aug 2, 2022

weird that this PR is still up

We don't need to list more package-managers. If you have installed Arch, how could it be possible that you don't know how to use pacman

certain distros have mpv under different names, shouldnt be an issue as it is trivial to figure out what the package is named in ur distro. flatpak is mostly encouraged anyways.

i think this should suffice most of the linux users. i dont want to be teaching a gentoo user how to install mpv.

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.

5 participants