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

Jump to bottom

treewide: src cleanup #102396

Merged
merged 1 commit into from Nov 2, 2020
Merged

treewide: src cleanup #102396

merged 1 commit into from Nov 2, 2020

Conversation

prusnak
Copy link
Member

@prusnak prusnak commented Nov 1, 2020 •
edited

Motivation for this change
  • replaced repo = "${pname}"; with repo = pname;
  • replaced rev = "${version}"; with rev = version;
  • used fetchFromGitHub where possible

I tested that none of the changes causes rebuilds meaning my changes were pure refactors.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@prusnak prusnak mentioned this pull request Nov 1, 2020
Closed
10 tasks
@jonringer
Copy link
Contributor

jonringer commented Nov 1, 2020 •
edited

doing something like:

rev = "refs/tags/v${version}"; into rev = "v${version}";

is tricky, as a repository could both have a branch, and a tag with the same name. I would move those changes to a different PR.

The other changes (e.g. repo = "${pname}"; with repo = pname;) are not very controversial

@ofborg ofborg bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: python 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 labels Nov 1, 2020
@prusnak
treewide: simplify rev/repo arguments in src
041bc79 
+ use fetchFromGithub where possible
@omasanori
Copy link
Contributor

doing something like:

rev = "refs/tags/v${version}"; into rev = "v${version}";

is tricky, as a repository could both have a branch and a tag with the same name. I would move those changes to a different PR.

How about adding tag to fetchers, migrating to it where appropriate, and revising rev to accept revision-like strings after a sufficient migration period? It would be better if we could save Nix writers from such situations IMHO. It will be larger and longer-term task which seems a drawback though.

@prusnak
Copy link
Member Author

prusnak commented Nov 1, 2020

Removed controversial changes in 041bc79

jonringer
jonringer approved these changes Nov 2, 2020
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

LGTM

https://github.com/NixOS/nixpkgs/pull/102396
48 packages built:
crackxls ddate eolie gx gx-go libmkv lumo matrix-synapse mirage-im mopidy-iris mopidy-local mopidy-mopify mopidy-mpd mopidy-mpris mopidy-musicbox-webclient mopidy-somafm mopidy-soundcloud mopidy-spotify-tunigo mopidy-tunein mopidy-youtube orion pantalaimon python27Packages.hawkauthlib python27Packages.pyfxa python27Packages.pykka python27Packages.tokenlib python27Packages.unpaddedbase64 python37Packages.hawkauthlib python37Packages.matrix-nio python37Packages.pyfxa python37Packages.pykka python37Packages.pyramid_hawkauth python37Packages.signedjson python37Packages.tokenlib python37Packages.unpaddedbase64 python38Packages.hawkauthlib python38Packages.matrix-nio python38Packages.pyfxa python38Packages.pykka python38Packages.pyramid_hawkauth python38Packages.signedjson python38Packages.tokenlib python38Packages.unpaddedbase64 retdec retdec-full tbe weechatScripts.weechat-matrix xwinmosaic

@jonringer jonringer merged commit 2c93131 into NixOS:master Nov 2, 2020
@prusnak prusnak deleted the src-cleanup branch November 2, 2020 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonringer jonringer jonringer approved these changes

@FRidh FRidh Awaiting requested review from FRidh

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@mmahut mmahut Awaiting requested review from mmahut

Assignees
No one assigned
Labels
6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: python 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@prusnak @jonringer @omasanori