Attendees: @edolstra @roberth @ericson2314 @thufschmitt @fricklerhandwerk @tomberek @infinisil
Notes: @infinisil @fricklerhandwerk
Agenda
- review re-requested after long wait: https://github.com/NixOS/nix/pull/8678
- @tomberek and @tfc were working on string performance at Ocean Sprint
- @fricklerhandwerk got feedback from contributors about the Nix team
- funding development time
- long-standing PRs, review turnaround time, self-merges
- pair on reviews
Funding Nix C++ development time
- @tomberek and @fricklerhandwerk have talked with @tfc, who has with serious C++ expertise, about working on Nix for money
- Are we in agreement this is something we want to happen?
- @tomberek and @fricklerhandwerk willing to help manage that, but the funding process needs to be bootstraped
- Related: Let financial contributors choose to fund the Nix team specifically · Issue #8431 · NixOS/nix · GitHub
- A long time ago @shlevy was crowdfunded on an issue: Get rid of the Perl dependency · Issue #341 · NixOS/nix · GitHub
- Need to find the right things to work on
- Reuse parts of the STF store proposal
- Take inventory and document the intent of existing tests
- Split out libstore to be self-contained
- Improve factoring and in-code documentation
- Add C bindings to the store API
- Reuse parts of the STF store proposal
- Requires not only C++ knowledge but also Nix knowledge. Maybe start with an orthogonal task not requiring Nix knowledge.
- Not a big worry with @tfc.
- Could (should) the foundation board decide to kick off an initial funding?
- Board shouldn’t (won’t) make technical decisions, this also means no prioritisation of technical work
- The team should just ask for a budget, board has overview of where all the budget flows
- Idea: Propose to allocate say 50kEUR to community teams, to be requested by submitting a proposal
- Possibly cut up the budget in advance in coordination with community teams, but let each chunk sit until an implementation plan is provided by the respective team
- Decision: @fricklerhandwerk and @tomberek will write up a proposal to the foundation board
Feedback to the team from people at Ocean Sprint
The team appaers more responsive, and it feels like it’s possible to land changes. Yay!
String optimization effort by @tomberek
Some string operations are very frequent and more expensive than what they should be. @tomberek tried optimizing that, but harder than expected. @tomberek and @roberth will pair on that.
https://github.com/NixOS/nix/pull/8678
- Idea already approved, previously assigned to @edolstra
- Will look at it in the work part of the meeting
Backport FSInputAccessor and MemoryInputAccessor from lazy-trees by edolstra · Pull Request #9177 · NixOS/nix · GitHub
Merged 5 days after opening, without completed reviews, a lot of code, @roberth and @Ericson2314 not happy with the process.
Decision:
- Try it out: Wait for approval by at least one reviewer (not technically enforced to allow for quick trivial changes)
- Establish a routine to split out tangential discussion into issues
- Stay on the course towards working on smaller changes
- Finish review on this particular change after the fact
Complete discussion
- Merged 5 days after opening, without completed reviews, a lot of code
- @roberth and @Ericson2314 don’t approve
- @edolstra wanted to make progress, didn’t think it was a big problem, reviews are very detailed
- @Ericson2314: Should have the opportunity to discuss the design properly
-
@edolstra: Lazy tree backports create a lot of work if they stay open, needs a lot of rebasing
- @Ericson2314: I’d help with that
- @Ericson2314: It’s hard but that’s how big changes work in open source
-
@fricklerhandwerk: Let’s break down changes more if they don’t fly immediately as discussed recently
- @roberth/@Ericson2314: This PR looked well broken down, just needed more review time, still a lot of code
- @tomberek: Communication issue, whether self-merging is okay, to wait for review approvals, talk more
-
@roberth: At least one approving review needed
-
@edolstra: Don’t agree, stops progress, don’t want that for trivial PR’s
- @roberth: Trivial PR’s are still okay to self-merge
-
@fricklerhandwerk: It’s not about asking for permission if we agree on direction. Reviews are mainly for:
- Knowledge sharing of code, ideas and vision, both ways author ↔ reviewer
- Informing readers about the change
-
@edolstra: Don’t agree, stops progress, don’t want that for trivial PR’s
-
@infinisil: Review PR afterwards?
- @roberth: Yeah let’s review afterwards, no need to revert
-
@edolstra: The problem is not reviews per se, but many reviews devolving into architecture astronautics
- @roberth: Not everything that’s commented needs to be implemented. Review can be part conversation, part suggestion, part questions, etc.
- @fricklerhandwerk: Establish routine to split off discussions into issues if necessary
- @infinisil/@tomberek: Related: https://conventionalcomments.org/
- @edolstra: Have to see how it works in practice
- @Ericson2314: Let’s try it out
Split-up reviews
- @tomberek/@thufschmitt: Git smudging
- @Ericson2314/@thufschmitt: https://github.com/NixOS/nix/pull/8595
- @Ericson2314: Git hashing
-
@fricklerhandwerk: Overhaul completions, redo #6693 by Ericson2314 · Pull Request #8131 · NixOS/nix · GitHub
- merged
-
@fricklerhandwerk: https://github.com/NixOS/nix/pull/8595
- reviewed
That’s good to know. I was a bit afraid that the idea of nix profile as a whole might not be accepted.