-
Notifications
You must be signed in to change notification settings - Fork 41
Clean up minor issues, build system and documentation #57
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Suggested-by: yamlllint
Suggested-by: cppcheck
Since the parameters are unused, just prefix them with an underscore. Suggested-by: cppcheck
They don't create files so aren't real targets.
Some BSD install programs don't have the -D option to create the dir.
The file is already deleted and deletion was desired, so don't fail when the desired state was already present.
This will be helpful for distributions packaging the plugin. Use a symlink instead of installing a binary in the /etc directory, since that is meant to be configuration files only, unfortunately mpv does not load plugins from anywhere in the /usr directory tree. Support the DESTDIR variable for overriding the rootfs location and support overriding the PREFIX and the plugin directories.
This also works when using fakeroot and that will help distributions automatically install to the mpv system wide scripts directory.
The plugin should work on GNOME/KDE on the BSD disributions too, and the abbreviation "DEs" is not fully obvious at first glance.
Separate out the different parts of the process to use the plugin. Everyone will need to know what their system mpv needs before they can figure out if they will be able to use the plugin on their system, so put the information about that first. Most people (except some distro users) will need to know where mpv loads the plugin from, so put that before the install instructions. The install instructions are only needed once by most people, so move them down but before the build instructions. Mention the pre-built 64-bit Linux binary is for x86, since the 64-bit ARM architecture is becoming popular with Apple M1 etc. Add mention of the distro packages (via repology.org) as an install option and put it first, it is an option that many open source users prefer. The build instructions are needed by the fewest people so they go next, since there are distro packages and a pre-built 64-bit x86 Linux binary that people can try before they need to resort to building from source. Leave the D-Bus interfaces at the very end since most people except devs won't need to know the details and it is more technical documentation.
Thanks a lot! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.