Attendees: @fricklerhandwerk @ericson2314 (meeting in person at ZuriHac!) @thufschmitt @edolstra @roberth (later: @tomberek)
Agenda
- Determine priorities for work meetings
- Pair reviewing
Priorities for work meetings
We discussed a few approaches to give our continuous work a clearer direction. We generally agreed on reducing the number of open PRs, doing pair reviews in the work meetings, and marking PRs as draft if we request significant changes or cannot come to a conclusion quickly. A process for making design decisions more effectively and a policy for delegating authority on those decisions remain open questions.
Complete discussion
- looking at maintainers handbook
-
@fricklerhandwerk: predictability of getting non-trivial changes is probably the most important thing, given we have multiple far-reaching changes in the making
- @edolstra: cannot make guarantees without someone working on reviews continuously, we don’t have such a thing as paid employees
- @fricklerhandwerk: the fundraiser we discussed last time could help with that
- @ericson2314: churn through more PRs. fewer things open would make easier to keep an overview
- looking at CONTRIBUTING.md
- we’re already adding idea approved labels
- good first issues are probably not that helpful for our current issue, and can be addressed async
-
@fricklerhandwerk: if we had some front page, what would we communicate on it in terms of what is most worthwhile to work on?
-
@ericson2314: (structured) documentation
- may be working off narrow tasks that we provide
- or ideally designs to implement
- @thufschmitt: we should give high level design directions pull requests should follow
- @ericson2314: porting to new platforms, such as BSD
-
@roberth: changes that improve the code quality/readability
- refactoring eases testing (and vice versa)
-
@edolstra: ZMQ has a formal specification where the rule is that any PR not breaking the spec is more or less automatically accepted
- this of course doesn’t work here
-
@fricklerandwerk: what would be an approximation?
- @edolstra: the store model, the Nix language
- @fricklerhandwerk: Tvix tests against Nixpkgs as a proxy for a Nix language specification, and their store implementation tests against the Nix evaluator producing derivations and store objects
-
@fricklerandwerk: what would be an approximation?
- @fricklerhandwerk: we could highlight implementing RFCs, they are accepted and (theoretically) do not require design decisions on our end
-
@ericson2314: architecture documentation may help with answering this
- many contributions make things worse design-wise because they are not well informed
- this of course doesn’t work here
-
@ericson2314: (structured) documentation
-
@fricklerhandwerk: impulse question: what if we stop accepting PRs altogether until we clean up the situation?
- @edolstra: declaring a feature freeze is possible but stopping accepting contributions such as bug fixes would be overkill
- @thufschmitt: seems unreasonable, would scare people off, and we want to be contributor-friendly
-
@edolstra: could gamify this for ourselves and set attainable quantifiable goals
-
@ericson2314: if reviewing a PR gets to hard, mark it draft and move on
-
@fricklerhandwerk: sounds good, easy to implement as a policy
- could power through reviews in work meetings
-
@thufschmitt: but not as a whole group, rather pair reviews
- general agreement
-
@edolstra: the problem with reviews is not the review itself but making decisions
- need a policy
-
@fricklerhandwerk: sounds good, easy to implement as a policy
-
@ericson2314: if reviewing a PR gets to hard, mark it draft and move on
-
@ericson2314: contribution guides should make explicit that random new features are discouraged
- pull request descriptions should primarily say how a change fits in the concept, not why it’s a cool thing
- @fricklerhandwerk: this to some extent depends on making clear what the concept is
- pull request descriptions should primarily say how a change fits in the concept, not why it’s a cool thing
Reviews
Split out in pairs for reviews.
Crash during concurrent garbage collection · Issue #7370 · NixOS/nix · GitHub
- critical
- assigned to @edolstra for looking into it
Guard uses of lutimes, for portability by puffnfresh · Pull Request #7048 · NixOS/nix · GitHub
- reviewed by @ericson2314 and @fricklerhandwerk in breakout
- made draft, changes requested
Support opening local store with database on read-only filesystem by benradf · Pull Request #8356 · NixOS/nix · GitHub
- small change requests on error messages and documentation
fetchGit: allow fetching annotated tags by balsoft · Pull Request #6766 · NixOS/nix · GitHub
- reviewed by @thufschmitt and @tomberek in breakout