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

ani-cli: 4.8 -> 4.9 #366140

Closed
wants to merge 5 commits into from
Closed

ani-cli: 4.8 -> 4.9 #366140

wants to merge 5 commits into from

Conversation

Rishik-Y
Copy link
Contributor

@Rishik-Y Rishik-Y commented Dec 18, 2024

Adding support for 24.05

Tests: Ran it multiple times and is already present in NisOS 24.11

Package Information

  • Package name: ani-cli
  • Latest released version: 4.9 (for 24.05)
  • Current version on the unstable channel: 4.9
  • Current version on the stable/release channel: 4.8

Changes: Minor changes so Latest version works in 24.05 stable

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Maintainer ping:
@skykanin

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 18, 2024
@ofborg ofborg bot requested a review from skykanin December 18, 2024 22:49
@Rishik-Y Rishik-Y changed the title Update Ani-cli to latest version (For Nix 24.05) 4.8 -> 4.9 Ani-cli: 4.8 -> 4.9 Dec 19, 2024
Copy link
Member

@donovanglover donovanglover left a comment

Choose a reason for hiding this comment

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

Commit message should be ani-cli: 4.8 -> 4.9

Comment on lines 1 to 17
{
fetchFromGitHub,
makeWrapper,
stdenvNoCC,
lib,
gnugrep,
gnused,
curl,
catt,
syncplay,
ffmpeg,
fzf,
aria2,
withMpv ? true,
mpv,
withVlc ? false,
vlc,
withIina ? false,
iina,
chromecastSupport ? false,
syncSupport ? false,
{ fetchFromGitHub
, makeWrapper
, stdenvNoCC
, lib
, gnugrep
, gnused
, curl
, catt
, syncplay
, ffmpeg
, fzf
, aria2
, withMpv ? true, mpv
, withVlc ? false, vlc
, withIina ? false, iina
, chromecastSupport ? false
, syncSupport ? false
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the unnecessary formatting changes

Comment on lines 39 to 42
let
player = [ ] ++ lib.optional withMpv mpv ++ lib.optional withVlc vlc ++ lib.optional withIina iina;
in
[
gnugrep
gnused
curl
fzf
ffmpeg
aria2
]
++ player
++ lib.optional chromecastSupport catt
++ lib.optional syncSupport syncplay;
let player = []
++ lib.optional withMpv mpv
++ lib.optional withVlc vlc
++ lib.optional withIina iina;
in [ gnugrep gnused curl fzf ffmpeg aria2 ]
++ player
++ lib.optional chromecastSupport catt
++ lib.optional syncSupport syncplay;
Copy link
Member

Choose a reason for hiding this comment

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

If a formatter is used, make sure that you're using nixfmt-rfc-style

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay will do that in future! 👍️

pkgs/applications/video/ani-cli/default.nix Outdated Show resolved Hide resolved
@Rishik-Y Rishik-Y changed the title Ani-cli: 4.8 -> 4.9 ani-cli: 4.8 -> 4.9 Dec 19, 2024
@Rishik-Y
Copy link
Contributor Author

I Have made the necessary changes!
Could you please take a look at it now?

@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 19, 2024
Copy link
Member

@donovanglover donovanglover left a comment

Choose a reason for hiding this comment

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

24.05 is deprecated and ani-cli is already 4.9 in 24.11, so I think this PR should be closed

pkgs/applications/video/ani-cli/default.nix Outdated Show resolved Hide resolved
Co-authored-by: Donovan Glover <[email protected]>
@Rishik-Y
Copy link
Contributor Author

24.05 is deprecated and ani-cli is already 4.9 in 24.11, so I think this PR should be closed

Oh...
I was under the assumption that 24.05 was not depreciated
I am somewhat of a newbie here, so didn't wanna do changes in Master/unstable yet,
so to learn first i wanted to try in other stable version and i did get the response on not doing PR in 23.11 and older.
If what you said is true, please go ahead and close the PR.

@donovanglover
Copy link
Member

You can see the deprecated label on https://search.nixos.org/ and https://status.nixos.org/

For contributing to nixpkgs, it's ideal to target the master branch and use backport labels as appropriate.

@Rishik-Y Rishik-Y closed this Dec 19, 2024
@Rishik-Y Rishik-Y deleted the patch-2 branch December 19, 2024 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants