-
Notifications
You must be signed in to change notification settings - Fork 112
yazi: Add at v25.5.31 #6379
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
base: main
Are you sure you want to change the base?
yazi: Add at v25.5.31 #6379
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi thank you for trying to package this app. I must note that the usual workflow is the package request should be approved first before submitting a pull request. I will allow it this time but please keep that in mind.
As far the PR goes, this is a good first attempt. I have few notes:
- Did you initialize git hook before submitting this package? If you did, the hooks will format the message correctly. The correct format for the new package is:
packagename: Add at v1.0.0
- The policy is that 1 package should in on 1 commit. So you should squash (Search for
git squash
) your commits into one. - If a package depends on another package that is not already on repositroy, that dependecy is automatically approved for inclusion too. Ideally it would included in the same pull request as the main package/app. I see that
resvg
is needed foryazi
to display .svg file properly. - Please link the issue in the message if a pull request resolves something. In this case, this PR is resolving the package request. So, something like:
Resolves https://github.com/getsolus/packages/issues/6368
Thanks again for being interested contributing to Solus. Feel free to ask any question here or in Matrix packaging room
I appreciate the feedback! I believe I have made the appropriate changes and have squashed into one commit. |
Since resvg is optional, would it be best to have it as a separate package? Not all other distros have resvg and Yazi still works without it but since it could be added, I decided to try. |
bec3d5b
to
ee08893
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still few things I missed:
- Other distribution (Like Arch) ship
ya
binary too. Maybe we should do so. You can install multiple binary by passing the name of the binaries like this. - There are completions for bash, zsh and fish if
YAZI_GEN_COMPLETIONS=true
argument is passed on build steps. Example here. - Also install desktop file and icon in
assets/
to appropriate directory. Example here. There is a bit of problem tho regarding the icon file, Yazi ship a non-standard size. I usually just chuck the non-standard size icon file to/usr/share/pixmaps
but that directory is being phased out and should not be used anymore. You can use Imagemagick'smagick
command to resize the file dynamically like what they do on Arch.
I hope that is all. Thank you.
Thanks for the guidance. This should be closer to how Arch uses it and the icon and completion now appear. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, squash your commits again.
**Summary** Add Yazi, which is a blazing fast terminal file manager written in Rust, based on async I/O. Resolves: getsolus#6368
Summary
yazi: Add at 25.5.31
This is my first try at maintaining a package. Please let me know if anything needs to be added, changed, or deleted.
Resolves #6368
Checklist