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
ytalk: init at 3.3.0 #89857
ytalk: init at 3.3.0 #89857
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.
Thanks for the contribution :) I found a few things that needs to be fixed:
pkgs/applications/networking/instant-messengers/ytalk/default.nix
Outdated
Show resolved
Hide resolved
pkgs/applications/networking/instant-messengers/ytalk/default.nix
Outdated
Show resolved
Hide resolved
Thanks for the corrections. Yeah, taeer is me. I just forgot that I put my actual name as maintainer tag. |
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.
Looks good to me
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.
Missed one thing, please use pname
instead of name
pkgs/applications/networking/instant-messengers/ytalk/default.nix
Outdated
Show resolved
Hide resolved
Gotcha. Is that a new thing? |
Can you also make sure the PR is only one commit containing your changes? |
There is some discussion on the subject here NixOS/rfcs#35, not a standard it seems. |
Done! |
Oh shoot. I thought I was squashing but actually I was deleting the new commits. I'll fix that |
Okay, should be good now. Double check to make sure I actually included all of your fixes, since I had to re-do them. |
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.
uhm, can you remove the merge commit? rebasing the changes should be preferred. |
Oh, I thought this was already merged into nixpkgs. I'm not actually sure how to do what you're asking. I tried running |
Okay, I did something that I think worked? Not sure what I did exactly, but let me know if it worked or not. |
@Radvendii thx for the contribution. |
Motivation for this change
I wanted to be able to use ytalk
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)