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

Tracking issue: Removal of uneeded argument #313981

Open
Sigmanificient opened this issue May 23, 2024 · 13 comments
Open

Tracking issue: Removal of uneeded argument #313981

Sigmanificient opened this issue May 23, 2024 · 13 comments
Labels
5. scope: tracking Long-lived issue tracking long-term fixes or multiple sub-problems

Comments

@Sigmanificient
Copy link
Member

Sigmanificient commented May 23, 2024

Issue description

There are many unneeded argument that lies around due to the update/refactor made over the years.
Let's try to clean this up!

  • fetchpatch: 397
  • fetchurl: 81
  • fetchFromGitHub: 64
  • fetchgit: 16
  • fetchPypi: 13
  • fetchFromGitLab: 9
  • fetchNuGet: 5
  • fetchsvn: 2

I will break these down to chunk of 100 than a hundred file, to make the pr review process easier

Steps to reproduce

nix run nixpkgs#deadnix -- . -o json | grep 'Unused lambda pattern:'

Technical details

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-linux"`
 - host os: `Linux 6.6.30, NixOS, 24.05 (Uakari), 24.05.20240521.5710852`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.18.2`
 - channels(root): `"nixos-23.05"`
 - channels(sigmanificient): `""`
 - nixpkgs: `/nix/store/qzjidyx3fip413vg7by6ibl22lwizc68-source`
@isabelroses
Copy link
Member

@Sigmanificient is there anything I can do to help out here?

@Sigmanificient
Copy link
Member Author

Sigmanificient commented May 26, 2024

Still have fetchFromGitHub, and the others fetch* to remove (so 2 prs)

nix run nixpkgs#deadnix -- pkgs -o json | grep "Unused lambda pattern: fetch"
136

Deadninx reports 1577 others type of "Unused lambda pattern" which needs manual check to see what should be removed. They need sto be group int small chunks to make manageable prs

@isabelroses
Copy link
Member

Alright, if you don't mind I can take a shot at removing some fetchFromGitHubs.

@isabelroses
Copy link
Member

A little command I made along the way, this should get you a list of all the files that have any given type of the unused lamda pattern. nix run nixpkgs#deadnix -- pkgs -o json | nix run nixpkgs#jq -- -n '[inputs] | .[] | select(.results[].message == "Unused lambda pattern: fetchFromGitHub") | .file'

@Sigmanificient
Copy link
Member Author

Sigmanificient commented May 26, 2024

A little command I made along the way, this should get you a list of all the files that have any given type of the unused lamda pattern. nix run nixpkgs#deadnix -- pkgs -o json | nix run nixpkgs#jq -- -n '[inputs] | .[] | select(.results[].message == "Unused lambda pattern: fetchFromGitHub") | .file'

I used the following:

nix run nixpkgs#deadnix -- pkgs -o json \
    | grep "Unused lambda pattern: fetchpatch" \
    | cut -c 1-1000 | nix run github:Sigmanificient/filterpath \
    | xargs -i nvim {}

@rhendric
Copy link
Member

This is a good initiative, but big-change PRs tend not to survive because responsibility for the change is diffuse. Since we have an automated tool for detecting this, what if we rig a GitHub action to run it as part of PR linting, just on the files updated by that PR?

@Sigmanificient
Copy link
Member Author

Sigmanificient commented Jul 17, 2024

This could be a good idea, do you want me to pr a GitHub action checking this? I am not sure how to deal with some possible edge-cases that sometimes get reported by deadnix

@rhendric
Copy link
Member

I'd be happy to review it if you did!

@Sigmanificient
Copy link
Member Author

Do you know a good way retrieve changed files on a pr?
I currently use https://github.com/tj-actions/changed-files/releases/tag/v44 but it tries to unshallow the git history... which of course is going to take ages: https://github.com/Sigmanificient/nixpkgs/actions/runs/9966727273/job/27539264058?pr=2#step:4:1

@rhendric
Copy link
Member

https://github.com/karpikpl/list-changed-files-action is the first thing I found; looks sane enough, but I haven't used it.

@Aleksanaa
Copy link
Member

@Sigmanificient
Copy link
Member Author

Sigmanificient commented Jul 27, 2024

I think that there are still quite a few around, only removed a couple hundreds 😅 (~1700)

@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/we-added-a-linter-to-nixpkgs-checking-workflows/49722/24

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5. scope: tracking Long-lived issue tracking long-term fixes or multiple sub-problems
Projects
None yet
Development

No branches or pull requests

6 participants