I created a pull request a few weeks by, got rejected the first review, the second time the bot’s task queues… forever? Am I doing something wrong? Request to add codelldb binary
People might be busy. Afaik they’re trying their best but stuff can lag sometimes. (Btw I believe there is a problem with your link, I think it’s about this one).
Oops, you’re right, your link is correct. The thing is, last commit only took a week but the second is almost three weeks and nothing. Is this normal? Should i do something?
I give up. I deleted the branch and pr.
Don’t worry about it, all reviewers know to ignore stuck ofBorg tasks for Darwin. If there’s something wrong with your code, we will tell you about it.
(Most of us do actually have a bad habit of also telling you about things that aren’t strictly wrong but suboptimal, undesirable or even personal preference. If you get the feeling some request is actually in that category, feel free to press the reviewer on whether their request is necessary or merely an improvement they’d like to see.)
Unfortunately yes. Keep in mind that we’re just a whole bunch of volunteers who are in this out of their own interests.
There’s like 6000 open PRs and adding a vscode debugging extension(?) isn’t exactly the highest priority. It’ll take a bit.
People will eventually come around to it though if you stick through with it and it’s actually a sensible change that people would want.
Kindly ask for another review. Don’t be annoying of course but nobody would object a friendly reminder every week or so and if they don’t feel like reviewing your package anymore, they can always say so and unsubscribe.
Us people with merge privilege usually don’t know squat about the thing being packaged or changed but we need to somehow tell whether the change is any good. We usually only hit the merge button if we’re confident that the change is indeed good; we don’t want to merge bad code that doesn’t actually work. When it’s stuff we personally use, we can easily test this out ourselves of course but if it isn’t, it’s up to you to demonstrate to us that your code is good.
You can achieve this by:
- Providing smoke tests as either quick commands we can run or, ideally, NixOS tests.
- Describing how and to what degree you tested your own changes.
- Getting other users of the component to try your changes and report what they found; leaving a review comment on the PR stating that what they tested and how well it works.
6 Likes
Yes, you’re absolutely right. An extension that’s used by very few people is probably not worth submitting. I do remember reading something like “It’s not worth adding the package if there’s only like 5 people who uses this” in the nix repo. I have moved the package to my own repo, where it should be. Thank you for the detailed answer and explanation, and I am deeply sorry for wasting your time.
The point was more that the less people use it, the less priority it gets in reviewing, not that it shouldn’t be submitted.
But yes the PR review process sucks overall and usually is the worst for new contributors.
My approach in the past was just to send the PR and forget about it completely, if it gets attention weeks/months later then great but sometimes it just sits there forever.
Read again, that’s not what I said.
I said that it’s unlikely to be prioritised but that’s not due to it not being a welcome addition but rather that there are so many other changes to more important (or, realistically, interesting) PRs that committers will focus on first. It’ll just take a little longer.
Also, I don’t believe this is used by very few people as it has 2.5k stars on GH which (unless bought) is a good indication that it’s a project that’s actually used by a good amount of people, no?
Don’t fret to send out a few friendly pings every now and then or ask another committer you know (i.e. me) to take a look. Often times things just fall through the cracks or get forgotten about. We’re people too.
1 Like
To be clear I wasn’t looking for advice for myself, I just pointed out that it’s not unusual to not get a review for weeks/months, and I was suggesting that newer contributors avoid getting too… emotionally invested in one PR, since that will lead to frustration.
(It also helped that most of my PRs were not for software that I personally used, and I use a long-running nixpkgs fork for my own configs, so I wasn’t too bothered if it was merged or not.)
I have also tried to ping individual committers, in the past, with just about no positive effect. I think one of my PRs even had 7 committers on it, and was unmerged for over a month, until an 8th somehow found the PR and merged it.
Sorry to misinterpret. The project itself is widely used, and has already been packaged in nixpkgs, as my pr stated. My goal was to specifically expose the codelldb binary, so that other editor with a DAP client could utilize this vscode plugin as well(in my case, neovim). So the case is actually neovim+dap protocol+cpp/rust debugging+codelldb. This is a very very specific use case, and maybe not a lot of people is interested in this. Sorry for the wording earlier, nonetheless, I have read again.
That doesn’t sound very niche to me tbh. We have stuff that is so much more niche than this in Nixpkgs haha 
Yes, I understand. The reason I am so frustrated is that I use the package everyday. Before it got merged, I’ll have to rely on a local copy for the binary, which is outside nix store, meaning my nix config depends on a random file in some random dir. Not reproducible. Since I am on nix-darwin, everytime I update my OS to a new beta, the binary is listed as “dangerous” and I have to go to settings to allow this. Each time there’s more than a single binary, since ALL of its lib files are listed as “dangerous”, they also had to be marked as safe manually as well. You can imagine the frustration everytime I get to work after an update, I have to spend time just approving the files.
You don’t have to do that, you can put the nix expression in a file and callPackage it.
(pkgs.callPackage ./yourfile.nix {}) can be used directly wherever you’d use any other package in your config.
Though I don’t use darwin so I’m not familiar with those specific issues.
1 Like
Thank you for the suggestion. That is a good idea indeed. I just thought merging is fast, so I thought: just use this unreliable bit for a day or two before I can get to it, it’s fine. And I just never used callPackage or anything like that.