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.

Sign up for GitHub

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

bat: add meta.mainProgram #246943

Merged
merged 1 commit into from
Aug 3, 2023
+2 −1
Merged

bat: add meta.mainProgram #246943

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pkgs/tools/misc/bat/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ rustPlatform.buildRustPackage rec {

src = fetchFromGitHub {
owner = "sharkdp";
repo = pname;
repo = "bat";
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a specific reason for this change? repo = pname seems to be a very common pattern

Copy link
Contributor Author

@drupol drupol Aug 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

repo = pname; is nice for DRY but creates a binding that goes too far.

See further information about this here: nix-community/nixpkgs-lint#21

Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIK finalattrs hasn't been implemented for buildRustPackage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sadly not yet indeed.

Copy link
Contributor

Choose a reason for hiding this comment

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

So there isn't a reason for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this is still valid, even without finalAttrs pattern. If one needs to overrides pname, it will also have an impact on the src. This change is preventing that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I won't ask you to revert it but frankly this change is pointless.

I don't want to loose too much time on this

I'd ask you to consider that other people may not want to loose too much time on stuff like this either.

This PR could have been a one line diff adding mainProgram that could have been merged without discussion as soon as it passed ofborg.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm dedicating my time for free for this project and every time I edit a PR, I'm trying to use what the "seniors" has been telling me for months when I was submitting my first PRs.

Constantly refining for a greater good the things that I'm "touching", I see that it is not very welcome, point noted, maybe I should avoid dedicating time to this community?

rev = "v${version}";
hash = "sha256-cGHxB3Wp8yEcJBMtSOec6l7iBsMLhUtJ7nh5fijnWZs=";
};
Expand Down Expand Up @@ -58,6 +58,7 @@ rustPlatform.buildRustPackage rec {
homepage = "https://github.com/sharkdp/bat";
changelog = "https://github.com/sharkdp/bat/raw/v${version}/CHANGELOG.md";
license = with licenses; [ asl20 /* or */ mit ];
mainProgram = "bat";
maintainers = with maintainers; [ dywedir lilyball zowoq SuperSandro2000 ];
};
}