Live data from Hacker News

Making a PR to Nixpkgs

johns.codes

1–10 of 22 posts

Re: Making a PR to Nixpkgs

#3

I’d do a git commit —amend instead of a rebase to keep the number of commits to 1

That is what I dislike about the Pull Request workflow. It encourages piling on commits, polluting commit log (not to mention useless merge commits). To this day I haven't found anything better than Gerrit for reviewing changes across iterations. I wish it makes a comeback on broader scale.

Re: Making a PR to Nixpkgs

#4

I’d do a git commit —amend instead of a rebase to keep the number of commits to 1

Ideally your commits are squashed and then merged anyway, GitHub just still uses merge as a default since I assume it's the most "logical" to git newbies.

Re: Making a PR to Nixpkgs

#5
post #3

I’d do a git commit —amend instead of a rebase to keep the number of commits to 1

That is what I dislike about the Pull Request workflow. It encourages piling on commits, polluting commit log (not to mention useless merge commits). To this day I haven't found anything better than Gerrit for reviewing changes across iterations. I wish it makes a comeback on broader scale.

Pull requests are great, you just need to be mindful of what commits you're sending people. Every commit I put in a PR is a commit I made with intent. Just squashing them all does them a disservice.

Re: Making a PR to Nixpkgs

#6
While I really love nix and nixos, I must say that Nixpkgs development is just out of control. There are over 5k issues and almost 4k PRs open. Reviewers are far fewer in number than contributors and there are multiple instances of low-quality packaging. Some features that most beginners want (for example, easily packaging nodejs programs) are still not there. Lack of standard is a heavy issue and there are many ways of doing the same.

Re: Making a PR to Nixpkgs

#7
post #6

While I really love nix and nixos, I must say that Nixpkgs development is just out of control. There are over 5k issues and almost 4k PRs open. Reviewers are far fewer in number than contributors and there are multiple instances of low-quality packaging. Some features that most beginners want (for example, easily packaging nodejs programs) are still not there. Lack of standard is a heavy issue and there are many ways…

I agree with what you’re saying, but I have to say that using `npmlock2nix` was really easy. Things are changing and improving all the time, which is good but also creates these sorts of problems.

Re: Making a PR to Nixpkgs

#8
post #3

Earlier quoted context omitted.

That is what I dislike about the Pull Request workflow. It encourages piling on commits, polluting commit log (not to mention useless merge commits). To this day I haven't found anything better than Gerrit for reviewing changes across iterations. I wish it makes a comeback on broader scale.

Pull requests are great, you just need to be mindful of what commits you're sending people. Every commit I put in a PR is a commit I made with intent. Just squashing them all does them a disservice.

I don't know how many times I've said that reviewing the structure of the commits is part of a PR, just like reviewing the code. It seems to be a revelation.

Re: Making a PR to Nixpkgs

#9
post #3

I’d do a git commit —amend instead of a rebase to keep the number of commits to 1

That is what I dislike about the Pull Request workflow. It encourages piling on commits, polluting commit log (not to mention useless merge commits). To this day I haven't found anything better than Gerrit for reviewing changes across iterations. I wish it makes a comeback on broader scale.

luckily github can just squash it for you if you choose. You don't have to have a useless merge commit. The option is right there in the dropdown. I assume gitlab as well.

Re: Making a PR to Nixpkgs

#10
NixOS is the most daunting Linux distribution ever, and all the people I know that use it are extremely smart. You need to develop a really good mental model of how this complicated web of Nix scripts works, to create packages.
Post reply on HN