<ma27>
does anybody want to review the following (relatively small) nix pr: https://github.com/NixOS/nix/pull/2266 (it would be awesome to see this in one of the next releases :) )
pie__ has quit [Remote host closed the connection]
pie__ has joined #nixos-dev
init_6 has joined #nixos-dev
eadwu has quit [Ping timeout: 250 seconds]
asymmetric has quit [Ping timeout: 250 seconds]
eadwu has joined #nixos-dev
eadwu has quit [Ping timeout: 245 seconds]
eadwu has joined #nixos-dev
eadwu has quit [Read error: Connection reset by peer]
eadwu has joined #nixos-dev
orivej has quit [Ping timeout: 244 seconds]
lassulus_ has joined #nixos-dev
pie__ has quit [Remote host closed the connection]
pie__ has joined #nixos-dev
lassulus has quit [Ping timeout: 250 seconds]
lassulus_ is now known as lassulus
eadwu has quit [Ping timeout: 246 seconds]
worldofpeace has quit [Ping timeout: 246 seconds]
sir_guy_carleton has quit [Quit: WeeChat 2.2]
Cale has quit [Ping timeout: 260 seconds]
Cale has joined #nixos-dev
init_6 has quit [Ping timeout: 240 seconds]
init_6 has joined #nixos-dev
pie__ has quit [Remote host closed the connection]
pie__ has joined #nixos-dev
init_6 has quit [Ping timeout: 246 seconds]
init_6 has joined #nixos-dev
init_6 has quit [Ping timeout: 250 seconds]
FRidh has joined #nixos-dev
phreedom has quit [Ping timeout: 256 seconds]
pie__ has quit [Ping timeout: 252 seconds]
init_6 has joined #nixos-dev
orivej has joined #nixos-dev
orivej has quit [Ping timeout: 246 seconds]
orivej has joined #nixos-dev
<timokau[m]>
>1k open pull requests o.O
<Profpatsch>
oops
<Profpatsch>
timokau[m]: What’s the problem for that?
<Profpatsch>
Congestion of maintainers? Like far too few people are mentioned to have time to act/review on thees?
<Profpatsch>
Or just that nobody is mentioned, so PRs are not reviewed at all?
<Profpatsch>
We have a big systemic problem.
<Profpatsch>
(to be honest I invest exactly 0 time reviewing PRs I’m not mentioned in)
<Profpatsch>
gchristensen is working on adding maintainer mentions to ofborg, so that’s a good first step.
<Profpatsch>
But is that enough?
orivej has quit [Ping timeout: 268 seconds]
<bennofs[m]>
gchristensen: if we added the builds.json to the channel binary cache, then that bloom filter would be easy to build...
<timokau[m]>
Profpatsch: I can't say for sure. I still hope my review rfc (https://github.com/NixOS/rfcs/pull/30) could help, but nobody got around to implementing the necessary ofBorg support yet
<arianvp>
building rocksdb works in nix-shell but fails in nix-build
<Profpatsch>
timokau[m]: idk, that sounds like a technical solution to a social problem, namely: people only have so much time.
orivej has joined #nixos-dev
<Profpatsch>
If we assume every maintainer is willing to spend 1 hour per week of their free time to review and merge stuff, we need at least 20–30 active maintainers I’d assume.
<Profpatsch>
And that’s just for small version upgrades.
<FRidh>
pretty sure that currently way more than 30h per week is put in on reviewing PR's
<Profpatsch>
FRidh: Probably, yes. But do you think most open PRs take more than 10 minutes to review?
<Profpatsch>
I’d wager there’s 90% trivial stuff and only small amounts of non-trivial PRs.
<Profpatsch>
And focus should be put on merging all the trivial stuff in less than a week at least.
<Profpatsch>
Because otherwise contributors get discouraged.
<FRidh>
Profpatsch: depends on what you want to cover in your review. Small version bumps can typically be handled in a couple of minutes, tops. Most PR's are like these. While trivial, I am not sure whether we should spend too much time on these. Those wanting a rolling release can put in that effort. I don't think it's worth it, despite wasting a lot of time on them.
<Profpatsch>
FRidh: I agree that updating stuff on-demand is more worthwhile than being always on the newest release.
<Profpatsch>
But I guess most first-time contributions are trivial patches like these, and if nobody reacts we just lost a potential contributor.
orivej has quit [Ping timeout: 240 seconds]
init_6 has quit []
FRidh has quit [Read error: Connection reset by peer]
orivej has joined #nixos-dev
orivej has quit [Ping timeout: 240 seconds]
<arianvp>
Profpatsch: do you think automation will help?
<arianvp>
gchristensen: the bloomfilter idea is neat
<arianvp>
is that to stop incurring costs of slow 404s?
<gchristensen>
yeah
<gchristensen>
not sure it is actually worth it, but yeah
<arianvp>
It's an elegant solutionb
<arianvp>
but is 404s actually that expensive currently?
<Profpatsch>
arianvp: idk, automation only helps when it removes steps of work people are already doing.
sir_guy_carleton has joined #nixos-dev
<Profpatsch>
Automating something that is not already practiced is normally not as effective.
<arianvp>
I think the bloom filter is also useful for local development. say when im in a plane. such that nixos wont try to fetch things that are defenitely not there
<Profpatsch>
arianvp: So e.g. if we say “we want to make sure that every first-time-contributor (under 10 commits) gets priority reviews and is invited to do maintenance on their 10th commit” then somebody needs to start doing it manually.
<arianvp>
because now if I work say on a local haskell project, I always manually need to enable local builds
<arianvp>
but with bloom filter, we could make that decision automatic, and it would be sound
<Profpatsch>
And if it’s a worthwhile thing, then we can automate (e.g. by adding tags and creating a list of issues and/or notifications).
<gchristensen>
arianvp: do you know C++? :)
<arianvp>
barely
<gchristensen>
same :D
<arianvp>
I know C++ because I know rust
<arianvp>
:P
<arianvp>
but the Nix codebase seems pretty okay-ish
<gchristensen>
we'd need the bloom filter to be generated server-side in a serialized format -- not a list of paths
<arianvp>
sometimes I wish Nix was written in Go
<gchristensen>
the Nix codebase is pretty good c++
<gchristensen>
but we can sort out that part later, if support for loading a bloom filter per cache is added to Nix I can generate a serialized bloom filter
<arianvp>
shouldn't be too hard
<arianvp>
only have to edit BinaryCacheStore class I think
<arianvp>
and of course some tooling to generate the bloom filter?
<gchristensen>
well skipping the generation
<gchristensen>
just the part about Nix using it
<gchristensen>
so after a scientific look at 3 pull requests, ofborg's maintainer algo seems to be doing the right thing
<arianvp>
Profpatsch: another problem I see is that we never _close_ prs
<arianvp>
If a PR goes nowhere, and needs signifcant work to even be rebased on master, I Dont see why we shouldnt' close it
<gchristensen>
PR authors can reopen a closed PR, so I agree
<arianvp>
I think there's at least 100 PRs that could be closed for this reason
<arianvp>
Kubernetes has an auto-close bot on PRs for this
<arianvp>
>6 months inactivity && conflict with master => autoclose
<gchristensen>
interesting
<gchristensen>
that seems good
<arianvp>
and then the user can reopen it again if he found it important
<gchristensen>
where was that when I was looking for it!
<gchristensen>
😻
<timokau[m]>
For some reason not listed under pull requests
<gchristensen>
thank you timokau[m]!
<gchristensen>
"Note: You can now use emoji in label names, add descriptions to labels, and search for labels in a repository. See the blog post for full details. To access these features and receive payloads with this data during the preview period, you must provide a custom media type in the Accept header:" man they really phoned it in when writing this doc: unlinked from the pull page, bad copy :P
<timokau[m]>
Thank *you* for doing the hard part and actually implementing it
<timokau[m]>
Yeah I was wondering what the relevance of that note was
<gchristensen>
thatis copy-pasted from the beta note about adding emojis
<arianvp>
gchristensen: I dont have the writing skills to write a coherent RFC though
<FRidh>
or is it because the maitnainer is already pingen in the main msg?
<LnL>
I think that was intentional
<LnL>
(for now)
<gchristensen>
FRidh: right now it is is "no-op" mode, just posting the list in a Details link in the Checks
<FRidh>
ok
<gchristensen>
I didn't want to accidentally spam a million people
<FRidh>
makes sense
<gchristensen>
FRidh: it turns out first I have to extend the github api client to support review requests before I can do it, anyway :)
<FRidh>
gchristensen: instead of review requests, you already have the possibility to post a comment, right? Maybe that's sufficient for the next step.
<gchristensen>
that is a bit more complicated, because each PR is evaluated many times. each time, I'd have to scan each comment for people already mentioned and not mention them. it is doable, but a bit annoying
<gchristensen>
and I'm trying to get rid of comments :)
<FRidh>
Right. With such complications it's no longer a shortcut
<gchristensen>
and it is only about 100loc to add it to hubcaps :)
<LnL>
heh
<FRidh>
this is pretty useless: error: attribute 'pname' missing, at /home/freddy/Code/libraries/nixpkgs/pkgs/stdenv/generic/make-derivation.nix:84:54
<FRidh>
that's the whole trace
<FRidh>
so...which drv
<gchristensen>
where is that?
<FRidh>
a branch of mine. Made a bunch of changes, testing eval locally
<gchristensen>
ouch
<Profpatsch>
FRidh: no luck with --show-trace?
<FRidh>
Profpatsch: nope this was output using --show-trace
<FRidh>
Note I found which part of my changes is causing it. Looking now whether make-derivation can be improved here
sir_guy_carleton has quit [Ping timeout: 246 seconds]
<domenkozar>
gchristensen: I think elm development with Nix is a breeze now, if you ever consider returning to package browser rewrite :)
<gchristensen>
good news, y'all, ofborg will now ping maintainers on PRs where <10 maintainers are identified.
<LnL>
gchristensen++
<{^_^}>
gchristensen's karma got increased to 57
<domenkozar>
gchristensen++
<{^_^}>
gchristensen's karma got increased to 58
<gchristensen>
(at least, it will once the deploy finishes ... tick tock, buildkite!)
<timokau[m]>
gchristensen++
<{^_^}>
gchristensen's karma got increased to 59
<nbp>
For posteriryt, I am quoting my-self about fix-point: “This is like having a time machine to tell you what is the output, without introducing a paradox :)”
<nbp>
^ posterity
<gchristensen>
damn. you can only request reviewers of collaborators, so maintainers who aren't members can't be requested
worldofpeace has joined #nixos-dev
phreedom has joined #nixos-dev
<infinisil>
gchristensen: Huh, is this a limitation of github?
<gchristensen>
yeah
<infinisil>
Ohh yeah it doesn't work manually either
<gchristensen>
right
<gchristensen>
I have a solution in mind, but I'm still looking in to it
<domenkozar>
"quick, implement reviewing by reusing the assignment function, it's just a 1 day task"
<infinisil>
I wouldn't mind it being a comment with all the pings, similar to the previous bot
<domenkozar>
is how I imagine reviewing widget to appear :D
<gchristensen>
lol
<FRidh>
we could create a nixpkgs team with read-only access, and then with a little script and priviliged user add all package maintainers to it. Ugly but gets it further
<samueldr>
domenkozar: your scenario makes more sense than you give yourself credit for: only project members can assign reviewers
<samueldr>
a non-member contributor cannot ask for a reviewer directly :/
<gchristensen>
FRidh: exactly my thought :)
<samueldr>
an annoying bit with the R/O team is that everyone in the maintainers list will appear as "Member" anywhere in the NixOS org :/
<domenkozar>
samueldr: I know, the feature was never thought through UX wise on github side
<LnL>
if they don't use github there's no point in assigning it
<LnL>
oh, contributor is not enough?
<gchristensen>
right, they need to be a Collaborator
<LnL>
I get it now, got those confused as the same thing
<samueldr>
as far as github is concerned: user has a commit in repo? contributor and that's pretty much it
<samueldr>
and AFAICT there's no way to make some kind of additional level
<gchristensen>
well, we can add them to a team which grants them no special privileges
<gchristensen>
that is the next level, afaik
<samueldr>
yeah, let's add "without making them member of the org"
<LnL>
that's not possible I think
<LnL>
the workaround is becoming a member without actual permissions
<samueldr>
yeah, circling back to my initial "no one else is a bit concerned by the fact that "Member" will be shown on everyone in that team's comments and contributions?"
<gchristensen>
turning that on its head, how cool will it be to be a contributor and get to wear then nixos badge with pride
<samueldr>
I'm not sure there's concern to be had, but doubts persist
<samueldr>
yeah, that would be neat
<samueldr>
hmmm, that makes me think: epic does something similar
<gchristensen>
w.r.t. your concerns with coretemp, is your concern: they're rude on other projects and will carry the nixos symbol and reflect poorly on us?
<LnL>
so the "member" badge would imply maintainer, not committer
<samueldr>
yeah
<gchristensen>
or, is your concern: they're rude on our project and will carry extra weight as a Member when being rude to another contributor?
<samueldr>
gchristensen: more the latter than the former
<gchristensen>
well then we'd better strip them of their maintainership status
<gchristensen>
(conveniently, I had that answer ready for both ;))
<samueldr>
but my initial thoughts were less about inter-personal within nixpkgs, but more about external
<LnL>
if that's a concern we could make maintainers mean a bit more then it does now
<gchristensen>
I agree
<gchristensen>
I also think with the rolling out of this ofborg patch, being a maintainer *does* mean more
<gchristensen>
automatically
<LnL>
at the moment it basically means nothing
<LnL>
yeah, exactly
<samueldr>
and just to be clear: I'm not even sure there's concern to be had, but just thinking there could be ways to be abused, I'm sure more abuse could be thought of along the way
<samueldr>
and it all rely on how the principle of people are the softest link any social engineering attack :/
* samueldr
butchered that sentence
<samueldr>
it rlies on the principle, just as with social engineering, that people are the softest link (weakest if you prefer the proper word)
<samueldr>
though yeah, if Member is easy to come by, then it won't have as much meaning, so there wouldn't be any weight to abuse?
<samueldr>
and the benefits outweight the hypothetical non-issues